Page MenuHomeWildfire Games

Decouple PanelEntities code from session and use object orientation

Authored by elexis on Oct 22 2019, 6:27 PM.



This patch decouples the panel entity GUI code from the global session code and uses the class syntax to achieve that, refs #5387.

The code logic is changed a bit (while the behavior visible to the player is the same) in order to perform less useless work.

  • Buttons are created at one point in time and only updated with simstate relevant code.
  • The buttons are only repositioned if a button was inserted or deleted.
  • Each buttonhandler will use the same button throughout its lifetime rather than mapping dynamically.
  • sprite, onPress and onDoublePress are ressigned upon addition / removal of the button and perform the lookup on the entityID directly rather than iterating / lookup to find the correct entity
  • updateGUIStatusBar was reduced to 3 lines (changing one property of the size property) due to lack of other use cases and GUI resizing being simple enough to make the helper an unneeded indirection and odditiy instead of a timesaver
  • Engine.GetGUIObjectByName only upon construction of a button / manager to avoid repeated useless work
  • Health bar resizing only if health changed

These changes may help to make it easier to animate the button position instead of instant changes, as the TODO from rP18361 suggests.

The performance analsis shows that this is only slightly faster, < 5%, because there are two heavy performance blockers here.
One is the GetEntityState and the other is the tooltip functions (which also call GetEntityState).

Those two consume 300 microseconds per button on my machine whereas the rest of the new update function consumes something like 30 microseconds.
So the result is that we get yet another hint that GetEntityState interface needs to become smarter.

The tooltip functions equally. Perhaps one can make it so that the tooltip work is not performed anymore unless the tooltip is shown.
It would be a C++ patch, so thats only a prospect.

Everything besides those two evil calls should have been reduced to noisefloor, at least it's all 1, 5, or otherwise < 30 microseconds,
see the code comments in the first version of the patch uploaded.

Attached a measurement showing something like 13 relics in observerview selected, and the same replay from player 1 perspective who gained and lost relics and a hero.
(Notice the first graphics has a horizontal offset)

replay used to measure:

replay used to verify behavior:

Notice that it now uses 15 microseconds where it had 80 microseconds before in case there are no panel entities, yay.

Test Plan

Correctness of the patch is ensured by testing that ingame everything works as it should, i.e.

  • having relics and training a hero results in the hero being inserted first (defined order)
  • gaining more relics than slots are available does not result in an error (fixed by D1397/rP23074)
  • All items are shown to the observer view, but if a player is selected, only the entities of that player
  • Add warnings to insert and delete functions and see that they are only called if a panel entity changed ownership, thus ensuring to minimize performance impact. Thereby also make sure that there is no stupid insert+delete again bug every call if having reached the button limit.
  • Measure each individual JS statement to see the performance weighting and see that this is optimal if one does not want to touch GetEntityState or tooltip code.

Diff Detail

rP 0 A.D. Public Repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Oct 22 2019, 6:27 PM

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

Link to build:

Successful build - Chance fights ever on the side of the prudent.

Link to build:

elexis edited the summary of this revision. (Show Details)Oct 22 2019, 6:37 PM

The pattern is one that does some needless array construction.
However that is so little work that it makes no performance difference over a reduce call that creates the string directly:

		// 150 microseconds
		let foo = Engine.GetMicroseconds();
		if (false)
		this.panelEntButton.tooltip =
			this.nameTooltip + => tooltip(entityState)).filter(tip => tip).join("\n");
			let tooltip = this.nameTooltip;
			for (let tooltipHandler of this.Tooltips)
				let ttip = tooltipHandler(entityState);
				if (ttip)
					tooltip = "\n" + ttip;
			this.panelEntButton.tooltip = tooltip;
		warn(Engine.GetMicroseconds() - foo);

Here again the code with the comments that show the performance imbalance:

	update(i, reposition)
		// TODO: Instead of instant position changes, animate button movement.
		if (reposition)
			setPanelObjectPosition(this.panelEntButton, i, Infinity);

		// 150 microseconds
		let entityState = GetEntityState(this.entityID);

		// 20 microseconds
		if (this.hitpoints != entityState.hitpoints)
			let size = this.panelEntityHealthBar.size;
			size.rright = 100 * entityState.hitpoints / entityState.maxHitpoints;
			this.panelEntityHealthBar.size = size;

		// TODO: only  build tooltip if the object is hovered?
		// 150 microseconds
		this.panelEntButton.tooltip =
			this.nameTooltip + => tooltip(entityState)).filter(tip => tip).join("\n");

		// 1 microsecond
		if (this.hitpoints > entityState.hitpoints)
			startColorFade(this.overlayName, 100, 0, colorFade_attackUnit, true, smoothColorFadeRestart_attackUnit);
		this.hitpoints = entityState.hitpoints;
nani awarded a token.Oct 22 2019, 6:49 PM
elexis added inline comments.Oct 22 2019, 7:41 PM
70 ↗(On Diff #10193)

onPress supports the selection.add hotkey, onDoublePress does not, so adding it to the latter, including ResearchProgress.
After implementation I notice that every doublePress two press events are triggered, so screw that.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 22 2019, 7:54 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Oct 22 2019, 7:54 PM
Polakrity added a subscriber: Polakrity.

Nice works.
Very detailed demonstration.

elexis edited the summary of this revision. (Show Details)Oct 22 2019, 8:22 PM