Re: [Freeciv-Dev] Unreachableprotects for Capturing / Bombardment

2013-04-24 Thread Marko Lindqvist
On 25 April 2013 02:40, Emmet Hikory per...@shipstone.jp wrote:

 Looking at the UTYF_BOMBARDER nativity issue, I notice that GNA patches
 1850 (unreachableprotects) and 1851 (capturer/capturable) seem to have
 surrounded the prior bombarder code, with the effect that Unreachable
 units do not protect other units from being captured or bombarded,
 regardless of the setting.

 In the case of capturing units, there is a test to ensure every unit at
 the target tile is capturable, but it doesn't also check to see if the
 capturable units are reachable, so that if one had a capturable
 unreachable unit (e.g. Payroll plane), it could be captured by units
 that would normally be unable to target the class (e.g. Horsemen).

 In the case of bombardment, whether a given unit happens to be reachable
 is checked as each unit is bombarded, but the stack is always presumed to
 be vulnerable, regardless of the server setting.

 As 1851 postdates 1850 in initial envisionment (if not application), and

both postdate the bombarder code, I suspect these behaviours to be
 intentional, but thought it worth asking, as they seemed surprising to me.


 After so long time I can't say how long they had been in development
before being first submitted (timestamps in patches indicate that final
versions were created just before submitting, but that's the case with
almost all my patches). Given my usual workflow it's entirely possible that
despite them having consequtive patch numbers, they have been developet
separately even without realizing they could affect each other.
 Addition of unreachableprotects setting certainly was meant to keep old
hardcoded behavior when enabled for compatibility of old rulesets.


 - ML
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] Unreachableprotects for Capturing / Bombardment

2013-04-24 Thread Emmet Hikory
Marko Lindqvist wrote:
 On 25 April 2013 02:40, Emmet Hikory wrote:
 
  Looking at the UTYF_BOMBARDER nativity issue, I notice that GNA patches
  1850 (unreachableprotects) and 1851 (capturer/capturable) seem to have
  surrounded the prior bombarder code, with the effect that Unreachable
  units do not protect other units from being captured or bombarded,
  regardless of the setting.
 
  After so long time I can't say how long they had been in development
 before being first submitted (timestamps in patches indicate that final
 versions were created just before submitting, but that's the case with
 almost all my patches). Given my usual workflow it's entirely possible that
 despite them having consequtive patch numbers, they have been developet
 separately even without realizing they could affect each other.

Heh.  At least it's recent enough that the patch author is active on 
the mailing list :) Given the uncertainty, I'll consider the relation 
between capturable and unreachableprotects to be entirely coincidental. 
I want to look at the implications a bit more, but will probably submit 
a patch moving the capturer/capturable check inside the 
unreachableprotectsr+alliance guard (we don't really want to be capturing 
allied freight, etc.).

  Addition of unreachableprotects setting certainly was meant to keep old
 hardcoded behavior when enabled for compatibility of old rulesets.

I've just read through the ML traffic about PR#8455, but suspect 
I'll have to do that again (gmane threading doesn't work so well on deep 
archives) to determine the intent of the commit (as the intent of the 
original patch changed significantly during patch review).  I suspect that
I'll end up wanting to rewrite bombardment entirely, rather than just trying
to fix the nativity quickly, as there are so many deep assumptions buried
in the logic and functional composition.  Depending on the outcome of all
of this, it may or may not be moved inside the unreachableprotects guard.

-- 
Emmet HIKORY

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev