Page MenuHomeWildfire Games

Remove visibleclasses from guiinterface
ClosedPublic

Authored by bb on Sat, Sep 5, 9:29 PM.

Details

Summary

The visible classes are uselessly cloned in the guiinterface. This has been since the introduction in rP15195. They have never been used, and never have been needed, hence we should nuke them in favor of performance.

Test Plan

grep and see nothing breaks
Observe the front not falling
Look at the old commit and agree on uselessness
Look at the other values cloned in guiinterface.GetEntityState and see they are used (still some maybe shouldn;t be there, but simply removing them will error out)

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

bb created this revision.Sat, Sep 5, 9:29 PM
bb edited the test plan for this revision. (Show Details)
Vulcan added a comment.Sat, Sep 5, 9:31 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3149/display/redirect

Vulcan added a comment.Sat, Sep 5, 9:33 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1497/display/redirect

Vulcan added a comment.Sat, Sep 5, 9:33 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2596/display/redirect

bb requested review of this revision.Sat, Sep 5, 9:33 PM
bb updated this revision to Diff 13422.Sat, Sep 5, 9:36 PM

They were used in the tests...

Or make it useful by using sim-information in the tooltip? Although I guess PetraAI would break if one changes classes in-game.

Angen accepted this revision.Fri, Sep 11, 9:58 PM
Angen added a subscriber: Angen.

removing visibleclasses from guiinterface is reasonable
tooltips get data directly from template
petra does not call guiinterface
I did not check usefulness of other things in getentitystate

This revision is now accepted and ready to land.Fri, Sep 11, 9:58 PM
bb added a comment.Sun, Sep 13, 1:17 PM

Or make it useful by using sim-information in the tooltip? Although I guess PetraAI would break if one changes classes in-game.

Note too that the current implementation is broken: from the template we store it under visibleIdentityClasses. Hence the patch letting classes be modifyable, or the patch wanting classes in the simstate, should properly implement it.

I did not check usefulness of other things in getentitystate

I did read through everything and did some greps. Didn't find more useless stuff.

This revision was automatically updated to reflect the committed changes.