Buildings can be placed even if the specified population cost exceeds the available population slots. This was probably not noticed before as building population cost remains as 0 in all vanilla buildings.
Details
- Reviewers
- None
Set pop cost of a buildings to something and see if its possible to exceed the population available. Alternatively build some "Gohma eggs" from the Hyrule Conquest mod.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 6403 Build 10609: Vulcan Build Jenkins Build 10608: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [MAJOR] ESLintBear: | | Parsing error: Unexpected token for binaries/data/mods/public/simulation/helpers/Commands.js | 53| var·g_Commands·=·{ | | [NORMAL] JSHintBear: | | 'g_Commands' was used before it was defined. binaries/data/mods/public/simulation/helpers/Commands.js | 541| » » » » ····&&·player·!=·+cmd.owner) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js | 729| » » » » var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGUIInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | 'array comprehension' is only available in Mozilla JavaScript extensions (use moz option). binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | Expected 'for' and instead saw 'id'. binaries/data/mods/public/simulation/helpers/Commands.js | 950| » » for·(var·i·=·0;·i·<·length;·++i) | | [NORMAL] JSHintBear: | | 'i' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 963| » » var·count·=·0; | | [NORMAL] JSHintBear: | | 'count' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1110| » » var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGuiInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1374| » » var·piece·=·pieces[j]; | | [NORMAL] JSHintBear: | | 'piece' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1457| » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1495| » » » &&·cmpFormation.GetMemberCount()·==·formation.entities.length) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js |1521| » » » » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1554| » » » var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation); | | [NORMAL] JSHintBear: | | 'cmpFormation' is already defined.
Link to build: https://jenkins.wildfiregames.com/job/differential/746/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [MAJOR] ESLintBear: | | Parsing error: Unexpected token for binaries/data/mods/public/simulation/helpers/Commands.js | 53| var·g_Commands·=·{ | | [NORMAL] JSHintBear: | | 'g_Commands' was used before it was defined. binaries/data/mods/public/simulation/helpers/Commands.js | 541| » » » » ····&&·player·!=·+cmd.owner) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js | 729| » » » » var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGUIInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | 'array comprehension' is only available in Mozilla JavaScript extensions (use moz option). binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | Expected 'for' and instead saw 'id'. binaries/data/mods/public/simulation/helpers/Commands.js | 950| » » for·(var·i·=·0;·i·<·length;·++i) | | [NORMAL] JSHintBear: | | 'i' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 963| » » var·count·=·0; | | [NORMAL] JSHintBear: | | 'count' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1110| » » var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGuiInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1374| » » var·piece·=·pieces[j]; | | [NORMAL] JSHintBear: | | 'piece' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1457| » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1495| » » » &&·cmpFormation.GetMemberCount()·==·formation.entities.length) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js |1521| » » » » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1554| » » » var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation); | | [NORMAL] JSHintBear: | | 'cmpFormation' is already defined.
Link to build: https://jenkins.wildfiregames.com/job/differential/747/display/redirect
This file should be minimized, perhaps an || would already do, one could compare if its benefitial to move the condition to one of the components, also the inequation looks wrong, as it's equivalent to cmpPlayer.GetPopulationLimit() > cmpPlayer.GetPopulationCount().
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [MAJOR] ESLintBear: | | Parsing error: Unexpected token for binaries/data/mods/public/simulation/helpers/Commands.js | 53| var·g_Commands·=·{ | | [NORMAL] JSHintBear: | | 'g_Commands' was used before it was defined. binaries/data/mods/public/simulation/helpers/Commands.js | 541| » » » » ····&&·player·!=·+cmd.owner) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js | 729| » » » » var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGUIInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | 'array comprehension' is only available in Mozilla JavaScript extensions (use moz option). binaries/data/mods/public/simulation/helpers/Commands.js | 900| » var·ids·=·[·id·for·(id·in·members)·]; | | [NORMAL] JSHintBear: | | Expected 'for' and instead saw 'id'. binaries/data/mods/public/simulation/helpers/Commands.js | 950| » » for·(var·i·=·0;·i·<·length;·++i) | | [NORMAL] JSHintBear: | | 'i' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js | 963| » » var·count·=·0; | | [NORMAL] JSHintBear: | | 'count' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1110| » » var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface); | | [NORMAL] JSHintBear: | | 'cmpGuiInterface' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1363| » » var·piece·=·pieces[j]; | | [NORMAL] JSHintBear: | | 'piece' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1446| » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1484| » » » &&·cmpFormation.GetMemberCount()·==·formation.entities.length) | | [NORMAL] JSHintBear: | | Misleading line break before '&&'; readers may interpret this as an expression boundary. binaries/data/mods/public/simulation/helpers/Commands.js |1510| » » » » » var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI); | | [NORMAL] JSHintBear: | | 'cmpUnitAI' is already defined. binaries/data/mods/public/simulation/helpers/Commands.js |1543| » » » var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation); | | [NORMAL] JSHintBear: | | 'cmpFormation' is already defined.
Link to build: https://jenkins.wildfiregames.com/job/differential/749/display/redirect
Just to reiterate the proposed change:
if the population limit has been reached, further house building would be disabled, right?
Actually, no. This only affects when buildings which have a population cost is being built. Currently an unused feature in the game.
Why would the check not consider the population that the building costs? (As mentioned perhaps one could move it to the components, i.e. to TrySubtractResources, but didn't check all occurrences)
Unlike a unit, the entity (foundation) is already created before placing it. So this.popUsed from player.js would include the building’s population count. This is why it makes moving it to the component a bit more complicated. It doesnt work the same way as units.
What about not creating the entity prematurely? Would make the code more consistent and reusable.
One might wanna see templates/special/filters/foundation.xml first.
Also, the patch is not like how it should be. One should just not create instead of destroying it in such circumstances.