As in autostart, the numplayers argument is only needed for random maps with more than 2 players, it is easily forgotten and this lead to some not easy to debug errors in the rmgen code. So it may be useful to have a clear error messages in such case.
Details
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 4810 Build 8344: Vulcan Build (Windows) Jenkins Build 8343: Vulcan Build Jenkins Build 8342: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Since 2 is the default, the condition is correct.
Keeping 2 as the condition as opposed to simplifying it and making it always mandatory is in line with the rest of the options which also come with defaults.
Are you sure you don't want to even throw PSERROR_Game_World_MapLoadFailed("hello world"); like is done
source/ps/GameSetup/GameSetup.cpp | ||
---|---|---|
1118 | here | |
1227 | and here | |
1281 | and here |
I'm not sure that patch is still needed as i can't anymore reproduce the original problem. Did you change the way PlayerData are used in random scripts?
Don't recall any such change. In fact I had removed the default fallback civ athen somewhere which can bug in Atlas in some way iirc.
But the gamesetup.cpp code looks like it handles the case of too many civs or AIs given already, so it doesn't appear like it was needed at any time this release?
18133 sanderd17 // Instead of overwriting existing player data, modify the array 9699 ben // Instead of overwriting existing player data, modify the array 13278 wraitii // Instead of overwriting existing player data, modify the array 15761 ben // Instead of overwriting existing player data, modify the array
I couldn't reproduce it either:
playa22 -autostart="random/alpine_lakes" -autostart-civ=1:athen -autostart-civ=2:brit -autostart-ai=2:petra -autostart-civ=3:athen playa22 -autostart="random/alpine_lakes" -autostart-civ=1:athen -autostart-civ=2:brit -autostart-ai=2:petra -autostart-ai=3:petra
Sounds like abandoning the patch should be done.
I just found that passing autostart-civ= results in an out-of-memory when parsing the JSON
source/ps/GameSetup/GameSetup.cpp | ||
---|---|---|
1357 | This part extends the object | |
1374 | looks like 2 should have would have to be replaced with numPlayers, so we don't have to keep it in sync | |
1375 | more than 2 AI players | |
1415 | So does this. |
AI, AI-Difficulty, Teams, Civ do extend the playerData array and those seem to be all player commandline options.
@mimo do you recall how you reproduced an issue originally or can imagine a way how it could be achieved?
The only thing I can think of is maybe passing player 1 and player 3 but not player 2.
Ah yes, i forgot about that one.
The problem is still there in fact if you forget the Civ attribute (not tested for the others).
Just launch an autostart with 4 players without the autostart-autoplayer: if you give the Civ for all, it will work, but if you forget it for any player, it will crash (without using the default arg) while when autostart-autoplayer is given, it will use the default civ.