HomeWildfire Games

Cleanup of GetEntityState
AuditedrP20875

Description

Cleanup of GetEntityState

Reviewed By: elexis

Trac Tickets: #4322

Differential Revision: https://code.wildfiregames.com/D1223

Details

Auditors
temple
Langbart
Committed
mimoJan 15 2018, 10:27 PM
Reviewer
elexis
Differential Revision
D1223: Cleanup of GetEntityState
Parents
rP20874: Add randomAngle helper function to abbreviate calls.
Branches
Unknown
Tags
Unknown
Build Status
Buildable 4488
Build 7844: Post-Commit Build

Event Timeline

elexis added a subscriber: elexis.Jan 16 2018, 2:59 AM
elexis added inline comments.
/ps/trunk/binaries/data/mods/public/gui/session/session.js
202

GetMultipleEntityStates can return null, but null can't be frozen. As reported by temple, this throws an error in some situations (presumably when having an entity selected that got destroyed).

Could just make that g_EntityStates[item.entId] = item.state && deepfreeze(item.state);

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_Operators

&& Returns expr1 if it can be converted to false; otherwise, returns expr2.

elexis added inline comments.Jan 16 2018, 5:21 PM
/ps/trunk/binaries/data/mods/public/gui/session/session.js
790

The code in Imaroks multi-selection patch primarily uses entStates.some.
So if 200 units are selected and and all loops abort after the first some iteration,
the 199 other entity states were not be used in that code part.
Whether this scenario is actually occuring depends on the entity templates and the most recent fancy multi-selection GUI features.
Just wanted to say that the GetEntityState function will always remain subject to optimization and revision as long as it's low performant.

(It would be nice if the tooltip functions could specify themselves which info they will need and then acquire exactly the used properties in the GUI interface)

temple raised a concern with this commit.Jan 16 2018, 11:19 PM
temple added a subscriber: temple.
temple added inline comments.
/ps/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
363–364

These used to be in the regular interface rather than miraged, so now when selecting we get:

ERROR: JavaScript error: simulation/components/GuiInterface.js line 363
TypeError: cmpFoundation.GetBuildRate is not a function
  GuiInterface.prototype.GetEntityState@simulation/components/GuiInterface.js:363:19
371

Same, I assume.

This commit now has outstanding concerns.Jan 16 2018, 11:19 PM
mimo added a comment.Jan 17 2018, 12:23 AM

Yes, thanks for the report. I'll have a look.

temple accepted this commit.Jan 23 2018, 5:45 PM
All concerns with this commit have now been addressed.Jan 23 2018, 5:45 PM
Langbart raised a concern with this commit.Sat, Nov 20, 2:25 PM
Langbart added a subscriber: Langbart.

A problem has been detected - #6387.

This commit now has outstanding concerns.Sat, Nov 20, 2:25 PM
Langbart resigned from this commit.Sun, Nov 21, 10:55 AM

this is not reason for the issue, see ticket for infos.

All concerns with this commit have now been addressed.Sun, Nov 21, 10:55 AM