Unable to upgrade some units.

[Dark0-04]

Is there an existing issue for this?

• [X] I have searched the existing issues

Game Version

4.9.1

Describe the bug

Units requiring a building cannot be upgraded, even when the required technology is unlocked and the necessary building is constructed. The bug only affects the Alpha Frontier mod.

Steps to Reproduce

1. Create a game with Alpha Frontier
2. Train a Rover Explorer
3. Research “Robotics” technology and build a Rover Storage.
4. Upgrade the already existing Rover Explorer : the upgrade button does not exist.
5. Note that you can still train Combat Rovers, but existing Rover Explorers cannot improve. Also note that the bug seems to affect all units that require a building, almost half of the units.

Expected Behavior

The button “Upgrade to Combat Rover” should appear

Screenshots

Link to save file

No response

Operating System

Android

Additional Information

Bug already reported on discord and also spotted by other people

[Dark0-04]

On my screens, I also use the Deciv in Alpha Frontier mod, but the bugs are also present on the original version of Alpha Frontier ! [Uploading Screenshot_20231127_211618_Unciv.jpg…]()

[Dark0-04]

On the screenshots, I also used the "Deciv in Alpha Frontier" mod, but the bug also appears on the original version of Alpha Frontier.

[SomeTroglodyte]

... and how do you figure this is a bug in Unciv's code, not the mods?

[yairm210]

Definitely an Alpha Frontier issue, not a general Unciv issue In fact, copy of #https://github.com/carriontrooper/Alpha-Frontier/issues/60

[SomeTroglodyte]

TLDR: Mod Error as in, not respecting the unwritten rule that OnlyAvailableWhen in the Unit upgrade code will only function with Conditionals testing the civ or unit itself, no others. Or - our error if we were to say we want that combo supported.

Long version: This raises questions...

• "Only available <in cities with a [Rover Storage]>" may work for city construction, but for unit upgrade the StateForConditionals used will not have knowledge about any city and thus the conditional will always fail.
• We have no declarative compatibility matrix which Conditionals can work on which UniqueTypes, thus RulesetValidator cannot catch this.
• Could we envision a UniqueType/UniqueType(target=Conditional) support level matrix? My gut says - too complicated, but then we'll always have modders misunderstanding a Unique/Conditional. The new freeform field docDescription might help - but in this case it would have to be on the OnlyAvailableWhen, saying "if applied to units, the upgrade action will only be able to check with Conditionals based on civ or unit"...
• Or, we could go back to the actual conditionalsApply test throwing an Exception if the state doesn't include a field required to evaluate a filter...
• In this case, one could easily build a richer StateForConditionals with city (unit.getTile().getCity()) and tile. Then, one could do units only upgradeable while sitting on an Oasis and only constructible in cities sitting on an Oasis... Not too helpful that last part.
• I would actually be tempted to make StateForConditionals a private constructor non-data class, and force instance creation through Companion factories. Or through secondary constructors. fun forUnit(unit: MapUnit) = StateForConditionals(unit.civ, unit.getTile().getCity(), unit, unit.getTile()) vs. constructor(unit: MapUnit) : this(unit.civ, unit.getTile().getCity(), unit, unit.getTile()). Then go through all compilation errors replacing until clean. Result: Inclusion of related knowledge inferred from one game object instance is centralized and can be adjusted consistently for all uses... Opinions?
• That change would mean a 'Rover Explorer' can upgrade as long as it's sitting on a tile belonging to a city with the required Rover Storage. (alternate StateForConditionals construction ways could require the candidate to be sitting in the city if we wanted that) Tested:


[SeventhM]

...Or Alpha Frontier can use the Unbuildable unique. There's a slight bug there too, but it's definitely a faster PR to parse

[SeventhM]

For the record, imo the better solution is for the upgrade logic to know that city based should be considered irrelevant. Probably the easiest way actually should be that conditionals that relies on city data maybe should pass as true (as if they were never there). Idk if that'll have any consequences elsewhere. Though in this case, we happen to just have an alternative unique to make this a nonissue

[hackedpassword]

We have no declarative compatibility matrix which Conditionals can work on which UniqueTypes

Wow, the post I needed! Spent hoursssss deciphering unique-conditionals that look like they should work...

In general, a compatibility matrix would be a godsend for compatibility reference. These unwritten rules are doing neither the modder nor player any good. The Unciv documentation is great, but uniques behavioralisms could use a serious documentation injection. Idea:

The uniques page lists all individual uniques. As efficient and neat as it is to list each unique one time, because of fuzzy overlaps between uniques and conditionals (plus everything else), is it possible to have a drop-down compatibility revealer that then lists each specific compatible unique, for that unique?

Or:

Activate the wiki feature in the Unciv repo here, do the above, but in the form of say, Minecraft compatibility tables. Please look at that link, at the several tables shown. Unciv uniques have complexities that I believe can be mapped similar to how Minecraft has disambiguated N-dimensional assets and entity relationships.

[yairm210]

There are 2 different issues One us "what unique good on what object" and for that there is a dropdown The other is "what conditionals go on what unique types" and that isn't mapped, should be mapped in the enum, not a problem but yes something we'll need to put thought into

That then leads to:

• Displaying in wiki "this can apply as conditional to ..."
• UniqueValidator