Change Details

Well this was a bit of a rabbit hole. After deserialization, we need to recalculate territories. GetTerritoryPercentage (called by the stats manager) calls CalculateTerritories to do that if needed. However, that function checks if the player argument is outside the bounds of valid players in a roundabout way: by checking m_TerritoryCellCounts.size(). This happens to be set by CalculateTerritories, so we have a chicken and egg issue. Changing things like so seems reasonable given the comment below, and won't incur a large penalty performance. Incidentally, one might wonder why the whole thing still works after rP27673 as we no longer check for territory decay each turn, but the territory manager has the m_TriggerEvent logic to handle thse kind of big setup changes. Edit: and to clarify, the bug isn't in rP27673 itself, instead I think the code has always been buggy, but before rP27673 the path from Position -> territoryDecay -> Recompute territories probably happened earlier, so the bug was hidden. I don't think the earlier vs later happening is particularly buggy, as every relevant function should still have to recompute the thing in the first place.

Well this was a bit of a rabbit hole. After deserialization, we need to recalculate territories. GetTerritoryPercentage (called by the stats manager) calls CalculateTerritories to do that if needed. However, that function checks if the player argument is outside the bounds of valid players in a roundabout way: by checking m_TerritoryCellCounts.size(). This happens to be set by CalculateTerritories, so we have a chicken and egg issue. Changing things like so seems reasonable given the comment below, and won't incur a large penalty performance. Incidentally, one might wonder why the whole thing still works after rP27673 as we no longer check for territory decay each turn, but the territory manager has the m_TriggerEvent logic to handle thse kind of big setup changes. Edit: and to clarify, the bug isn't in rP27673 itself, instead I think the code has always been buggy, but before rP27673 the path from Position -> territoryDecay -> Recompute territories probably happened earlier, so the bug was hidden. I don't think the earlier vs later happening is particularly buggy, as every relevant function should still have to recompute the thing in the first place. -- Edit: after adding tests, found two other minor issues from FLOODFILL being exclusive of the starting tile. Fixes rP16933 and rP16692

Well this was a bit of a rabbit hole. After deserialization, we need to recalculate territories. GetTerritoryPercentage (called by the stats manager) calls CalculateTerritories to do that if needed. However, that function checks if the player argument is outside the bounds of valid players in a roundabout way: by checking m_TerritoryCellCounts.size(). This happens to be set by CalculateTerritories, so we have a chicken and egg issue. Changing things like so seems reasonable given the comment below, and won't incur a large penalty performance. Incidentally, one might wonder why the whole thing still works after rP27673 as we no longer check for territory decay each turn, but the territory manager has the m_TriggerEvent logic to handle thse kind of big setup changes. Edit: and to clarify, the bug isn't in rP27673 itself, instead I think the code has always been buggy, but before rP27673 the path from Position -> territoryDecay -> Recompute territories probably happened earlier, so the bug was hidden. I don't think the earlier vs later happening is particularly buggy, as every relevant function should still have to recompute the thing in the first place. -- Edit: after adding tests, found two other minor issues from FLOODFILL being exclusive of the starting tile. Fixes rP16933 and rP16692