While reviewing D2810, I noticed that many of the Interfaces queried with queryOwnerInterface don't have safeguards for nonexistance. Usually not a problem since they exists, however they need to be checked.
Details
Details
- Reviewers
Freagarach - Commits
- rP24050: Add safeguards for all queryOwnerInterface calls
grep and see all have safeguards now
Agree with the proposed safeguards
Observe effectively nothing changes
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 13024 Build 25719: Vulcan Build Jenkins Build 25718: Vulcan Build (macOS) Jenkins Build 25717: Vulcan Build (Windows) Jenkins Build 25716: arc lint + arc unit
Event Timeline
Comment Actions
Seems good overall
| binaries/data/mods/public/simulation/components/ProductionQueue.js | ||
|---|---|---|
| 451 | I feel like this should be a straight warning/error. Seems buggy, particularly since you've already subtracted resources. | |
| 827 | Likewise -> makes no sense to have a tech template if the owner has no technology manager | |
| 893 | samesies | |
| binaries/data/mods/public/simulation/components/Promotion.js | ||
|---|---|---|
| 88 | no player means owner is invalid so entity is mostly dead | |
| binaries/data/mods/public/simulation/components/ProductionQueue.js | ||
|---|---|---|
| 451 | A second look showed, we already check for cmpTechnologyManager in this.GetTechnologiesList, so strictly we don't need to check this. Don't really like querying the interface twice, but out of scope | |
| binaries/data/mods/public/simulation/components/Promotion.js | ||
| 88 | Afaik the template doesn't contain a player ID | |
| binaries/data/mods/public/simulation/helpers/Commands.js | ||
| 302 | not much to change here, since no cmpPlayerEntityLimits means no limits, hence we just pass by | |
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3086/display/redirect
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1434/display/redirect
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2533/display/redirect
| binaries/data/mods/public/simulation/components/Promotion.js | ||
|---|---|---|
| 88 | I meant using +template.Promotion.RequiredXp. But Angen may be right and it may not be needed. | |
Comment Actions
This is complete and correct (awaiting the answer).
| binaries/data/mods/public/simulation/components/ProductionQueue.js | ||
|---|---|---|
| 383 | Should one still be allowed to train a restricted entity when there is no IID_EntityLimits? To me that equals having a limit of 0 so we should return here. | |