Pickable/included civs + a few bugs#918
Pickable/included civs + a few bugs#918stavrosfa wants to merge 2 commits intoC7-Game:Developmentfrom
Conversation
Fix bug in .biq and .sav files where we would assign barbarians to another civ. Added barb camp tiles for .sav and .biq Fix bug when loading a .sav to assign the correct civ color
| // It's easier to do it like this, otherwise we need to manipulate the biq arrays, | ||
| // to check for playable players, offset array indexes by the difference, etc, | ||
| // as we are parsing the file, which is 10 times the hassle compared to this approach | ||
| for (int p = save.Players.Count - 1; p >= 0; --p) { |
There was a problem hiding this comment.
[SUGGESTION] There's probably a nice LINQ way to implement this loop. Modifying a collection you are iterating over is always a bit sketchy. I'd set the barbarians unpickable in a separate statement.
There was a problem hiding this comment.
Yeah, you are right, this looks a bit silly in retrospective
ajhalme
left a comment
There was a problem hiding this comment.
Looks reasonable. I might have gone for 'playable' over 'pickable', but maybe the latter is more explicit.
Playable is already used by the original game to mark those civs whose data is included in the game (I think I have left a comment somewhere, that's the case with Mongols in the Middle Ages scenario), regardless of whether they can be picked by the AI or a human, hence the name. I changed the for loop to a linq as suggested, and added a |
This PR's details in short:
Implemented pickable and included-in-game civs
Fix bug in .biq and .sav files where we would assign barbarians to another civ.
Added barb camp tiles for .sav and .biq
Fix bug when loading a .sav to assign the correct civ color
The camps and colors could have been added separately, but trey arerather minor changes, and since I already had some tests in place to also test them, I though I might as well include them here.
These are some features/fixes that I choose to introduce at this point, because I was working on something else entirely, but my unit tests kept breaking. The main reason was the Mongols being included as a playable player in the Middle Ages scenario (which they are not), which we weren't really accounting for, causing numerous issues.