Chained upgrades lead to actionenabler inconsistency
We could probably allow this use case by adding a new upgrade action that only does one step at a time.
Sveinung (not logged in because of hardware trouble)
Is it a problem at all? Seems to me, in the discussed case just the rules can be formed differently, like, NoUpgrade flag to X type and test in actor reqs. The upgrade tree may be more complicated, maybe it will need more flags. But I don't see any glitch in this case, what you ask that you get.
Reply To ihnatus
Is it a problem at all? Seems to me, in the discussed case just the rules can be formed differently, like, NoUpgrade flag to X type and test in actor reqs. The upgrade tree may be more complicated, maybe it will need more flags. But I don't see any glitch in this case, what you ask that you get.
The reason not to give X the NoUpgrade flag is that I want it to be possible to upgrade X to Y; just not to upgrade either X or Y *to Z*. I'm not aware of any way for reqs to examine the 'destination' unit-type of the upgrade, only the 'source'; and even if there were, AIUI that would prevent upgrading X to Y once Z has been invented.
Reply To ec429
The reason not to give X the NoUpgrade flag is that I want it to be possible to upgrade X to Y; just not to upgrade either X or Y *to Z*. I'm not aware of any way for reqs to examine the 'destination' unit-type of the upgrade, only the 'source'; and even if there were, AIUI that would prevent upgrading X to Y once Z has been invented.
Oh, yes, I missed things here. Currently, ATK_SELF actions don't have any valid target reqs. Probably we should support unit type as target requirement factor for this action. I still don't think though that checking obsoletion chain is a good idea.
So we have two different suggestions: 1) That we add a new action type 2) That we make the goal unit as the target for the action.
To me that latter seems semantically incorrect, in a way that might bite us in the future. This is (currently always) self-targeted action, but it still doesn't meant that the action gets done to the goal unit type.
It would nice to find a consensus soon, to still consider this even to S3_1 d3f (it's an existing real-life ruleset that needs this, which might increase want enough to make this acceptable)
Reply To cazfi
1) That we add a new action type
This would work for "A -> B -> C" where we would block update to C. As far as I can see it would have some implications on "A -> B -> C -> D" where we would want to block update to D (if we don't want that blocking, one can still use the current action type); update price of A -> C would go up, as it's more expensive to update A -> B & B -> C, and UI-wise user would need to do two update actions.
Regarding the A -> B -> C -> D case, I envisaged the user would do a single upgrade action, the game would figure out it could get as far as C, *then* calculate the price of that A -> C upgrade and perform it as a single step. So that for purposes of upgrades, it would be as though C had obsolete_by = "None".
Reply To ec429
the user would do a single upgrade action, the game would figure out it could get as far as C, *then* ... perform it as a single step.
I've now been thinking how this could work for nearly two hours, and don't really see a way. I've considered single "meta-action" that would run other actions. I've considered "virtual actions" that cannot be performed, but just lend their enabler type to other actions to conditionalize their effects. We specifically want to check action enablers for each step, not some enabler for the entirety. Those individual enablers are not about enabling/disabling the entirety (except the first one), but affecting its behavior. How to have just one (main) action despite these things seems quite a hard problem.
Not sure I quite understand the problem, but mechanically it looks like the way to do it would be to add an actionenabler check to can_upgrade_unittype() in common/unittype.c. (It might be necessary to plumb some additional info through to that function, which currently only gets the unit-type and not e.g. the unit, tile or city. And handle_unit_type_upgrade() in server/unithand.c might get a little interesting.)
Which I guess ends up being the "virtual actions" approach. What does that break?
This might have an effect on resolving #42666 (Lua API for upgrade actions)
Reply To cazfi
This might have an effect on resolving #42666 (Lua API for upgrade actions)
The API I've suggested there allows iterative lookup for upgrade target type using utype = utype.obsoleted_by and test for player:can_build_direct(utype), we can also add any check for each step (just unit:can_do_action(...) test is likely missing now but may be introduced elsewhere). So a user-defined action that works as described could be written, just AI/UI won't handle all the cases when it is disabled (unless we can also write action enablers in Lua that is rather dubious in seeable perspective, especially about executing that Lua at client side).
By all logic (cost formula etc.), upgrade action is not chained (X to Y to Z) but immediate X to Z, and probably should be left as such. In the case mentioned, may we just add the requirements for building Z negated to the upgrade enabler for X and Y?
As discussion goes forward, it's becoming apparent that this is not a simple issue to resolve. Not trying to squeeze to S3_1-d3f, then.
Reply To cazfi
I've considered "virtual actions" that cannot be performed, but just lend their enabler type to other actions to conditionalize their effects.
This should be doable with 3.3 internal actions, but remains a hard problem in 3.2. We can either postpone again, or go by simple step-enabler accepting issues in A -> B -> C -> D case. I'd say we postpone, and implement it properly.
In my ruleset, I have a NoUpgrade flag that controls the Upgrade action, so that some units can be obsoleted (to declutter the build list) without being able to be upgraded (which e.g. in some cases bypasses an impr_req on the new unit).
However, if I have three units X → Y → Z (where → represents obsolete_by), and Y has the NoUpgrade flag, an X can still be directly upgraded to Z; I don't see any way to work around this.
In my opinion, the Upgrade Unit action should check that the entire chain of obsolete_by would all pass the actionenabler individually (and if not, upgrade to the point where it fails, so that e.g. X can still upgrade to Y after inventing Z), rather than just looking at the endpoints.