[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Update of patch #3839 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Update of patch #3839 (project freeciv): Planned Release: = 2.5.0, 2.6.0 ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Update of patch #3839 (project freeciv): Status: In Progress = Ready For Test ___ Follow-up Comment #9: For the SDL client fix, it's mostly a random swipe to remove it from grep results. That function should really be rewritten entirely to do things correctly, and if the nativity issue is a good reminder about that, there's no reason to patch it away. From the name of the function to be patched this is affecting how dialog gets constructed. So, is do_paradrop() comment relevant here? It won't be called before player requests (maybe illegally) the paradrop action. Consequently, does this change mean that playher can try to paradrop even when it will not be possible? ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #10, patch #3839 (project freeciv): This change may increase the cases where a player may attempt an impossible paradrop, but it also reduces the cases where a player may not attempt a possible paradrop. While it makes no difference for classical rulesets, this code means that no UMT_SEA or UMT_BOTH unit with UTYF_PARADROP may paradrop at all for SDL players, and also means that players using the SDL client may not paradrop onto oceanic transport, regardless of the ruleset definition of paradrop_to_transport. The removal allows these behaviours at the cost of a network exchange potentially reporting Cannot paradrop for units attempting to do things like paradrop to non-native terrain without transport. Note that this network round-trip is currently present for the GTK clients (either library version: try paradropping into ocean with the classic ruleset). The GTK clients currently use can_unit_paradrop() as the only condition, with everything else handled as server response from do_paradrop(), which makes sense to me, but I didn't want to tear all the conditionals out of the SDL client because I don't use it, so might not notice if I broke something removing more than necessary. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #7, patch #3839 (project freeciv): - assess_danger() Use utype_can_take_over() to indicate danger to cities I think this one affects even standard rulesets, and is a bug in all branches (to be fixed in S2_4, S2_5, TRUNK). AI does not consider enemy Helicopters city capture threat. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #8, patch #3839 (project freeciv): assess_danger() now in bug #20785 further reduced patch attached. (file #17889) ___ Additional Item Attachment: File name: low-hanging-nativity-fixes-more-reduced.patch Size:2 KB ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #5, patch #3839 (project freeciv): base_assess_defense_unit() change now part of patch #3885 ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #6, patch #3839 (project freeciv): Reduced patch, no longer including base_assess_defense_unit() or kill_something_with() (file #17850) ___ Additional Item Attachment: File name: low-hanging-nativity-fixes-reduced.patch Size:2 KB ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #3, patch #3839 (project freeciv): kill_something_with() is the change I'm most worried about too. You just let more units (in non-default rulesets) to go on with the main codepath, and I've seen that code to crash when unit properties unexpected to original implementation encountered before. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #4, patch #3839 (project freeciv): Ah, in that case, let's assume the kill_something_with() hunk to be unsafe: there's enough other work to be done in that part of the code for complex nativity that it will surely be revisited. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Update of patch #3839 (project freeciv): Status:None = In Progress Assigned to:None = cazfi ___ Follow-up Comment #1: all fairly lightweight. My gut feeling disagrees here (= I want to investigate this more carefully instead of going fast forward). Value of this patch is in pointing out places that need to be changed, but I suspect that some of the cases are much more complex than straightforward changes presented. I may even end up splitting this to several tickets to investigate some of the cases in more detail. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
Follow-up Comment #2, patch #3839 (project freeciv): Heh, perhaps my definition of lightweight was adjusted by the remainder of the nativity stuff :) I've no objection to splitting this into bits: it was assembled from bits as I tried to clean up nativity everywhere: these were the ones that were tiny patches, showed no behaviour changes in autogame testing, and didn't trigger me to add TODO items reminding me to investigate later. For base_assess_defense_unit(), I'm very certain of the semantics, and there are a few other places in the code that have had that change made (this one appears to have been missed). For assess_danger_unit(), there are no precedents that I found, but the behaviour is precisely the same for rulesets without UMT_LAND units granted CanAttackNonNative (and the change seems sensible). For assess_danger(), I'm less certain, and it may be worth performing autogame testing with rulesets specifically designed to hit this condition (presence of UMT_LAND units with !CanOccupy and UMT_SEA units with CanOccupy). For kill_something_with(), I was pleasantly surprised not to have differences in autogame testing: I was expecting this to fall into the more complicated category. Of everything collected here, I'm least confident in this change. For dai_rampage_want(), I'm pretty sure the current code won't work usefully even in the presence of Big Land from the experimental ruleset, ignoring more interesting nativity (e.g. Hut on Mountains). That said, I'm not absolutely sure my solution happens to be correct. For the SDL client fix, it's mostly a random swipe to remove it from grep results. That function should really be rewritten entirely to do things correctly, and if the nativity issue is a good reminder about that, there's no reason to patch it away. ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #3839] Low hanging nativity fixes
URL: http://gna.org/patch/?3839 Summary: Low hanging nativity fixes Project: Freeciv Submitted by: persia Submitted on: Mon 08 Apr 2013 04:58:06 PM JST Category: general Priority: 5 - Normal Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: ___ Details: Some more collected nativity fixes, all fairly lightweight. My testing showed no changes to autogame results with these changes. Specific changes documented in the patch, but most of them are semantically obvious (and one has precedent in prior revisions), excepting the gui-sdl change, which replaces clearly incorrect code with slightly incorrect code and a FIXME note. From what I can tell, looking at UMT_*, is_sailing_unit() and is_ground_unit(), the rest of them require more extensive code changes, rethinking of unit handling in various contexts, or recovery of lost semantics (where something is agreed only appropriate in some circumstance, but this isn't documented, or the documentation is out of date: the most amusing being a temporary kludge added as part of work-in-progress in 2002). Those few I have attempted generate autogame divergence, and so I believe they are better discussed individually (or in parts, for extreme cases like aiferry.c, where there are many assumptions conflated in the code which may be best untangled with a series of patches over time). ___ File Attachments: --- Date: Mon 08 Apr 2013 04:58:06 PM JST Name: low-hanging-nativity-fixes.patch Size: 4kB By: persia http://gna.org/patch/download.php?file_id=17713 ___ Reply to this item at: http://gna.org/patch/?3839 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
