Details
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3202/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2649/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1550/display/redirect
thank you
binaries/data/mods/public/simulation/ai/common-api/entity.js | ||
---|---|---|
36 ↗ | (On Diff #13491) |
binaries/data/mods/public/simulation/ai/common-api/entity.js | ||
---|---|---|
36 ↗ | (On Diff #13491) | That doesn't quite preserve the behavior and I get cargo cult programming vibes from it. First of all, 0 is the only possible falsy value here. Assuming otherwise implies template validation is broken. Second of all, while null != undefined returns false, null !== undefined returns true, which means that this is legitimizing the existence of an obvious bug by choosing to accept the value null in the cost information and not treating the key as undefined. I guess this boils down to whether "metal": null should be accepted or rejected. The template schema seems to suggest it should be rejected. |
binaries/data/mods/public/simulation/ai/common-api/entity.js | ||
---|---|---|
36 ↗ | (On Diff #13491) | Forgot to put this part in, What's the direction here? It might seem trivial, but the last question is IMO a pretty important distinction. |
binaries/data/mods/public/simulation/ai/common-api/entity.js | ||
---|---|---|
36 ↗ | (On Diff #13491) | For the record, false is also possible. But I undestand your point. Also null should be threated as undefined. |
I do not see ai getting any valueable information knowing that some was defined as null instead of undefined.
metal: null equals to not including that line at all from simulation point of view so should be the same as `undefined'.
It is not only about cost information. This is generic function. If one would want to fix just cost, one would fix line in cost function as following:
ret[type] = +(this.get("Cost/Resources/" + type) || 0) as other "defaults" are set.
I am also not sure what you mean by direction.
Right, hence not having null be distinguished from null.
For the focusing of costs, that's just because the subject matter. But thinking in generic terms, there isn't much use having this be a strict check because 0 and false is the only case where this will be issue, where both of them need to be preserved. But, undefined vs null are the same as you just said. I just don't see the point of not treating null as undefined.