Opened 3 years ago

Last modified 3 years ago

#4869 new defect

Don't pretend that civs are optional

Reported by: elexis Owned by:
Priority: Should Have Milestone: Backlog
Component: Atlas editor Keywords:
Cc: Patch:


Currently the gamesetup always sets a civ and sets the player_defaults.json civ entry if the map didn't define one.

The commandline autostart Gamesetup.cpp has a hardcoded athen fallback if the user didn't set the civ.

rmgen/ code has an ugly "athen" FALLBACK_CIV string and uses that in every place that does something with civs. New patches like Phab:D900 also expect a civ argument and add usage of FALLBACK_CIV.

The Identity entity component and the Player component of player entities currently also expect the civ argument to be well defined. All simulation component uses of these simulation components also require the civ to be well-defined.

So it only seems consequential to actually require the Civ property to be well-defined in all ways to generate a gamesetup configuration.

This means FALLBACK_CIV of rmgen/ can be deleted and that
ultimately the civ checkbox in atlas must be removed (so that the civ dropdown value becomes mandatory).

As side-quest, Gamesetup.cpp should load player_defaults.json to remove the hardcoded athen, as mentioned in some line of code in that file.

If we ever consider making civs optional, a lot of simulation code will have to be rewritten. It seems much saner to let mods that don't come with civ choices to still implement their own neutral standard civ.

Change History (3)

comment:1 by elexis, 3 years ago

Reproduce undefined civ fallback warning in Atlas of r20503 and before by disabling the civ checkbox of a player and generating a random map (or messing with the player numbers).

(By always setting the civ property in the atlas map generation, the checkbox (that should eventually be removed) will be checked if it was unchecked before and the dropdown item will be set to the fallback civ)

comment:2 by elexis, 3 years ago

In r20512:

Delete FALLBACK_CIV "athen" and related code in rmgen/, refs #4034.

It was only triggered if a civ wasn't set in Atlas.
Because civs are expected to be well-defined everywhere, alwas set the property in Atlas too, fixing the non-GUI part of #4869.
Use the existing player defaults value then instead of hardcoding a fallback civ string.

comment:3 by elexis, 3 years ago

As far as I see none of the atlas player settings are optional, so all of the checkboxes in atlas should vanish. A player always has a name, team, color, pop limit and starting resources.

There is only the small catch that scenario maps can define resources for every player while random and skirmish maps specify one set of resources equal to all players. Related mourning in #4504.

Note: See TracTickets for help on using tickets.