[Patch] Remove AiProxy from non-AI (player only) games

[wraitii]

AiProxy is a component basically every entity has, that allows sending the game state to the AI. With a sufficient number of entities, it can take around 3/4/5ms per turn. While this is rather insignificant in the grand scale of things, it would be good to be able to turn that overhead off in MP games without AI players.

The attached patch (from this branch: ​https://github.com/wraitii/0ad/tree/AiProxyOptim) does this by removing the subscription at launch. Not that I do not believe it handles deserialization yet.

I'm curious whether you think this is a good idea or unnecessary code bloat.

[elexis]

It seems clean to me to only initialize AI components if they are used. Therefore the idea of the ticket would be correct the attached patch improvable. Ideally the AIProxy does not even exist in games without AI (or would that introduce too many checks?).

[wraitii]

I thought about deleting AiProxy entirely, but I think it introduced AIProxy-specific code inside the component construction process, which I didn't really like. This patch is quite removed from the actual infrastructure, which seemed cleaner. For all intents and purposes, however, making it unsubscribe from everything removes it from the game entirely - so in principle it should be equally fast. I coded this a month ago however so not entirely sure if that was my full reasoning.

[wraitii]

better patch for Itms

[mimo]

As we already disable some AIInterface functions in helpers/Player.js (line 171), it would be nice if both disabling would use the same logic to determine if we have no AI players in the game.

[wraitii]

Attached patch serializes the deafening (saw no other easy way to do this) and fixes mimo's remark. Also renames it to DeafenComponent.

[elexis]

Replying to wraitii:

deleting AIProxy entirely introduced AIProxy-specific code inside the component construction process, which I didn't really like

Agree, but there might be other ways to implement optional components. We have CComponentManager::ConstructComponent, how about adding DestructComponent for example?

Code style:

• UnsuscribeComponent is more sound than Deafening (the latter seems more related to ears)
• wiki:Coding_Conventions recommend to use the explicit type instead of auto
• Missing space after comma in L568, perhaps more
• About the comment:

  	/**
  	 * Tells the Component Manager to remove all subscriptions of a given component type,
  	 * effectively making it similar to altogether removing that component (but in a less intrusive way).
  	 * This is called in public mod for AIProxy in player-only games.
  	 * Since AIProxy can end up taking several ms (>3/4) per turn doing nothing useful otherwise
  	 */
  

  • effectively making it similar to, double relativizations conceding false advertizing should be avoided in code who should be as accurate as conceivable. If the component wasn't removed, it can still do things.
  • The performance numbers are kind of arbitrary, should be saved for the ticket. It is sufficient to mention that subscription removal can be used to remove AI components in games without AIs (simultaneously avoiding the explicit reference to the public mod and AIProxy)
  • I do not believe we actually: same as above. Also not believing X is different from believing not X. The comment leaves the impression of doubt, so the coder or reviewer should figure this out beforehand.
• Comments should start with capital letters (at least it is grammatically correct for sentences)

[mimo]

Replying to wraitii:

Attached patch serializes the deafening (saw no other easy way to do this) and fixes mimo's remark. Also renames it to DeafenComponent.

I was rather thinking to remove the disabling of AIInterface from Player.js and put everything in InitGame with something like (although I've not tested it, so it may not work and need to be improved)

	if (settings.PlayerData && !settings.PlayerData.some(v => v && !!v.AI))
	{
		Engine.QueryInterface(SYSTEM_ENTITY, IID_AIInterface).Disable();
		// add your removal of AIProxy components
		return;
	}
	let seed = settings.AISeed ? settings.AISeed : 0;
	cmpAIManager.SetRNGSeed(seed);
	cmpAIManager.TryLoadSharedComponent();
	cmpAIManager.RunGamestateInit();

so that all disablings/removals are done at only one place (and we also no more need your new Engine function HasActiveAIs)

[Imarok]

-> ​https://code.wildfiregames.com/D9

[elexis]

Backlogging due to lack of progress