EFT_UPGRADE_UNIT conflicts with ACTIVITY_CONVERT
lexxie9952 See https://github.com/Lexxie9952/fcw.org-server/commit/fc681654f36641031b5aa99863c7f005f7f9e1df
This has fixed it perfectly so far (but we do get LOTS of free testing.)
Reply To (Anonymous)
See https://github.com/Lexxie9952/fcw.org-server/commit/fc681654f36641031b5aa99863c7f005f7f9e1df This has fixed it perfectly so far (but we do get LOTS of free testing.)
Yes, that seems like the right way to do it. Can you provide an actual patch?
I don't know how, but I would if someone teaches me.
Reply To (Anonymous)
I don't know how, but I would if someone teaches me.
http://www.freeciv.org/wiki/How_to_Contribute#How_to_create_patch_file_with_Git might help, though it's for a bit different use-case (developing directly with freeciv.org repository).
Can you work with that, or do you need more detailed instructions?
So I created the patches myself. Compared to Lexxie's FCW changes:
- I saw no reason to check for the activity even before target unit type, so rearranged those checks (that's mostly a matter of taste, I think, not much functional difference)
- S3_1/master: Fixed compile of api_methods_unit_transform_problem()
Side remark: when I wrote API to unhardcode autoupgrade, I missed properties of unit activity. That can be helped by using user effects but probably we need one.
There is a bug that happens when EFT_UPGRADE_UNIT (Leonardo Workshop) upgrades a unit that is in the middle of ACTIVITY_CONVERT.
This bug happens in any rulesets that rely on ACTION_CONVERT actionenablers, as a way to unlock conditional special bonuses for converting units.
The sequence goes like this: 1. TURN 1. Player gives ACTION_CONVERT order to unit_type ALPHA to convert to unit_type BETA. Server accepts the order because actionenabler reqs were legal. 2. END OF TURN 1: Leonardo upgrades unit_type ALPHA to unit_type BETA. 3. TURN CHANGE. Beginning of T2. The ACTIVITY_CONVERT finishes on unit_type BETA. Unit BETA converts to target unit_type GAMMA. 4. The actionenabler reqs for BETA to convert to GAMMA were completely bypassed. ALPHA legally began ACTIVITY_CONVERT to upgrade to BETA, then EFT_UPGRADE_UNIT upgraded it, so now ACTIVITY_CONVERT finishes and converts to unit_type GAMMA while bypassing the reqs for BETA to convert to GAMMA.
REAL EXAMPLE from MP2 rulesets: 1. Foot units get what we call a "hand-me-down" bonus. Upon discovering Conscription, Phalanxes/Pikemen can convert to Musketeers for free. (The idea: upgraded Riflemen are giving their muskets away to the older obsolete units: there are no Phalanxes in 1850CE.) 2. Communist government has a special bonus that Riflemen may convert to Workers for free. ACTION_CONVERT requires Communism for Riflemen to do it. 3. A Monarchy begins converting a Phalanx to a Musketeer with the "hand-me-down" bonus. The Phalanx converts to Riflemen from Leonardo. It then finishes its ACTIVITY_CONVERT and becomes Workers even though the government is not communist.
At first to fix this bug, we tried adding { "Activity", "Convert", "Local", FALSE} to the reqs for effect_leonardos_workshop, but this just makes the reqs always evaluate false. The simplest fix to this bug is probably to let EFT_UPGRADE_UNIT look at the activity reqs without always throwing false whenever there is an activity req in the req vector.
Special thoughts: the CONVERT order is a special new feature that helps distribute the workload for fixing the "obsolete unit traffic jam" away from Leonardo wonders and the expenditure of large amounts of gold that discourage making era-realistic units in a certain era only because the leader is somehow prophetic to 500 years later when all these units are obsolete and aren't worth the cost to upgrade. ACTION_CONVERT allows rulesets to favour certain types of underprivileged units into getting free upgrades, regulated through special req conditions that have to be earned or unlocked through improvements, technologies, government types, etc. I believe that ACTION_CONVERT will be used this way more and more in future rulesets, because it kills two flies with one strike (eases upgrade traffic-jam and helps buff/balance certain types of units who were wrongly discouraged because of an upcoming new types whose gold-upgrade cost unrealistically discourages the current unit_type ancestor.) MP2 is the "guinea pig" ruleset experimenting how to enrich the game by distributing the upgrade workflow across more dynamics that hook into strategic holistic balance, and discovered this issue with CONVERT vs EFT_UPGRADE_UNIT, but as more rulesets look for solutions to the "obsolete unit issues", we expect more will be frustrated by this bug.
For now, FCW will have to hack-patch something into our server to prevent this. But we'd like to be aware of what upstream thinks is the "right" way to solve this issue, and perhaps we might submit the fix if we all decide what's the good right way to do it!