battlekeeper/arma3performanceextension

Arma3PerformanceExtension

arma3performanceextension's Issues

Refactor Code Duplication in fn_hideServer

Problem

The same chunk of code appears to be repeated four times with a few minor changes between them.

Arma3PerformanceExtension/addons/A3PE/functions/fn_hideServer.sqf

Lines 22 to 66 in 483859e

	_CheckedUnit = _x;
	_vis = 0;
	_selections = ["rightleg","leftleg","rightarm","leftarm","head"];
	if (!isPlayer _CheckedUnit) then {
	if (_ViewDistance > _player distance2D _CheckedUnit) then {
	if (_player distance2D _CheckedUnit > _ForceRenderDistance) then {
	if (isNull objectParent _CheckedUnit) then {
	{
	if (_vis == 0) then { // Skips if unit was already calculated to be seen
	_pos = _CheckedUnit selectionPosition _x;
	_pos2 = _CheckedUnit modelToWorld _pos;
	if (isNull objectParent _player) then {
	_visnum = [_player, "VIEW", _CheckedUnit] checkVisibility [eyePos _player, AGLToASL _pos2];
	_vis = _vis + _visnum;
	} else {
	_visnum = [(vehicle _player), "VIEW", _CheckedUnit] checkVisibility [eyePos _player, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if !(_ZeusCameraPos isEqualTo [0,0,0]) then {
	_visnum = [objNull, "VIEW", _CheckedUnit] checkVisibility [_ZeusCameraPos, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if (_IsConnectedUav select 1 != "" || _ShownUAVFeed) then {
	_visnum = [_UAV, "VIEW", _CheckedUnit] checkVisibility [getPosASL _UAV, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if (_HigherQualityAI) then {
	_visnumFront = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [0,5,1]), AGLToASL _pos2]; // front
	_visnumLeft = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [-5,0,1]), AGLToASL _pos2]; // left
	_visnumRight = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [5,0,1]), AGLToASL _pos2]; // right
	_vis = _vis + _visnumFront + _visnumLeft + _visnumRight;
	};
	};
	} forEach _selections;
	if (_vis == 0) then {
	_allObjsHide pushBack _CheckedUnit;
	} else {
	_allObjsShow pushBack _CheckedUnit;
	};
	} else {_allObjsShow pushBack _CheckedUnit;};
	} else {_allObjsShow pushBack _CheckedUnit;};
	} else {_allObjsHide pushBack _CheckedUnit;};
	}; //isPlayer

It makes the file take longer to read.
It increases the chance of a fix or change being made in some of the chunks and not being ported consistently to the others.

Suggestion

Extract that chunk out into its own function.
Have optional parameters or a setting that will control weather any of the special checks are used.

A3PE_HCOverride Race condition

Problem

When a headless client joins and runs init, it sets A3PE_HCOverride to true to prevent other HCs from double calculating.

Arma3PerformanceExtension/addons/A3PE/functions/A3PEInit.sqf

Lines 41 to 43 in b67f011

	if (EnableHCOverride && {!hasInterface && {!isDedicated && {!(missionNamespace getVariable ["A3PE_HCOverride", false])}}}) then {
	missionNamespace setVariable ["A3PE_HCOverride", true, true];
	missionNamespace setVariable ["A3PE_HCNetworkID", clientOwner, true];

However, if two headless clients join near the same time (within the latency from one to the server, and the server to it, plus some extra as setVariable has a lower network priority compared to other game functions) then both will see A3PE_HCOverride as false or nil. And they will then collide for setting A3PE_HCNetworkID.

Suggestion

• Have the server manually choose a headless client to do the raycasting.

Exit Early When checkVisibility is Above 0

Problem

Arma3PerformanceExtension/addons/A3PE/functions/fn_hideServer.sqf

Lines 34 to 54 in 483859e

	if (isNull objectParent _player) then {
	_visnum = [_player, "VIEW", _CheckedUnit] checkVisibility [eyePos _player, AGLToASL _pos2];
	_vis = _vis + _visnum;
	} else {
	_visnum = [(vehicle _player), "VIEW", _CheckedUnit] checkVisibility [eyePos _player, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if !(_ZeusCameraPos isEqualTo [0,0,0]) then {
	_visnum = [objNull, "VIEW", _CheckedUnit] checkVisibility [_ZeusCameraPos, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if (_IsConnectedUav select 1 != "" || _ShownUAVFeed) then {
	_visnum = [_UAV, "VIEW", _CheckedUnit] checkVisibility [getPosASL _UAV, AGLToASL _pos2];
	_vis = _vis + _visnum;
	};
	if (_HigherQualityAI) then {
	_visnumFront = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [0,5,1]), AGLToASL _pos2]; // front
	_visnumLeft = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [-5,0,1]), AGLToASL _pos2]; // left
	_visnumRight = [_player, "VIEW", _CheckedUnit] checkVisibility [AGLToASL (_player modelToWorld [5,0,1]), AGLToASL _pos2]; // right
	_vis = _vis + _visnumFront + _visnumLeft + _visnumRight;
	};

_vis is checked to see if it is 0, or non-0. This is boolean behaviour.
However, _vis is allowed to accumulate more even if it was already set above 0. The means that unnecessary checks are performed.
Every selection is still looped, although at least a check skips each iteration if _vis is non-0.

Arma3PerformanceExtension/addons/A3PE/functions/fn_hideServer.sqf

Line 31 in 483859e

if (_vis == 0) then { // Skips if unit was already calculated to be seen

Suggestion

Refactor the code to make _vis a boolean.
Use exitWiths and lazy code-evaluation to exit on the first visible position.

Example:

// Previous
if !(_ZeusCameraPos isEqualTo [0,0,0]) then {
  _visnum = [objNull, "VIEW", _CheckedUnit] checkVisibility [_ZeusCameraPos, AGLToASL _pos2];
  _vis = _vis + _visnum;
};
// Becomes
if (_ZeusCameraPos isNotEqualTo [0,0,0] && {
  [objNull, "VIEW", _CheckedUnit] checkVisibility [_ZeusCameraPos, AGLToASL _pos2] > 0
}) exitWith {_vis = true;};

The first check here needs to be deleted so that exitWith exits the loop.

Arma3PerformanceExtension/addons/A3PE/functions/fn_hideServer.sqf

Line 31 in 483859e

if (_vis == 0) then { // Skips if unit was already calculated to be seen

Headless Client Disconnecting&Reconnecting Mid-Mission Will Leave Players with Hidden Objects.

Problem

Once the A3PE_HCOverride has been set. It will not be un-set until a mission restart.

Arma3PerformanceExtension/addons/A3PE/functions/A3PEInit.sqf

Line 42 in b67f011

missionNamespace setVariable ["A3PE_HCOverride", true, true];

This will be an issue if the headless client decides to disconnect and reconnect mid-mission (An occurrence sometimes noticed on the Antistasi Offical servers). After the headless client leaves, the server will not take over as the A3PE_HCOverride is true and the server loop would have exited. When the headless client reconnects, it will not take over as A3PE_HCOverride would still be true.

Suggestion

1. Make a server loop run in the background
2. If it detects that A3PE_HCOverride is true and that the clienID in A3PE_HCNetworkID is not connected.
3. Set A3PE_HCOverride to false and restart the server process. Or try to find another connected Headless client and re-execute the init script on them so that they will take over.

Work-Around

The mission hoster or admins should know that this can happen. In that case, they should instruct all players to toggle their Performance addon off so that the objects will be visible again. However, players will no longer benefit from the previous performance gains.

_allObjsHide and _allObjsShow List all objects, rather than just the changes.

Problem

If an object was hidden or visble in the last check.
The next check determines that it should still be hidden or visible (no change).
The server or HC process will still add that object to the array _allObjsHide or _allObjsShow.
That object in the array will be sent to the client again unnecessary.
This data wasting will occur for every object whose visibility has not changed.

Suggestion

1. The headless client will first send the big arrays of which object should be hidden or visible.
2. After that, it should keep the two hashmaps per player that contained the previous state.
3. If the visibility state is the same, skip. If it is different, remove it from one hashmap and put it into the other. Then add to either _allObjsHide or _allObjsShow.

Local Objects Will Be Calculated And Sent To The Client

Problem

The headless client or server may catch local objects from either allUnits, allDeadMen, allPlayers, or vehicles.
They will then process this and then order the client to show or hide it.

Arma3PerformanceExtension/addons/A3PE/functions/A3PEInit.sqf

Lines 46 to 49 in b67f011

	_allUnits = allUnits;
	_allDeadMen = allDeadMen;
	_allPlayers = allPlayers;
	_vehicles = vehicles;

Suggestion

fn_hideServer should first check the that netID of the object is not "0:0" before processing it further. Biki createVehicleLocal

Only one random HC can process objects.

Problem

Only one headless client can perform calculations.
For large missions with many players to calculate for, it would be nice to be able to specify multiple machines to cut up the work.
Which could include a mix of multiple headless clients and the server.

The headless client chosen may also be used by another script, forcing it to be shared while another HC could lay dormant.

Suggestion

• Have a tick-box to allow the server to take some of the load.
• Have a CBA text setting where the advanced server admin can type in the names of the Headless Clients that should be used. Otherwise, by default, it will just pick one at random.
• Each headless client will have an array local to it in missionNamespace which lists all the clients that it will calculate for.
• The server will have a background process that loops every minute or so and assigns new players to available headless clients. This should try to keep the load evenly. It should Redistribute clients if a headless client disconnects and reconnects mid-mission. It should keep the redistribution of clients to a minimum, so that headless clients don't need to re-calculate data that they have cached about the client (Relevant in a future issue, will be inserted.)

Performance Toggled Off Shows All Objects

Problem

Arma3PerformanceExtension/addons/A3PE/functions/fn_toggle.sqf

Lines 6 to 20 in 483859e

	[] spawn {
	sleep 0.5;
	{
	_x hideObject false;
	} forEach allUnits;
	{
	_x hideObject false;
	} forEach allDeadMen;
	{
	_x hideObject false;
	} forEach allPlayers;
	{
	_x hideObject false;
	} forEach vehicles;
	};

• It does not respect other scripts or initial mission creation that may have hidden the objects on purpose, as it will force show them.

Suggestion

1. Make a list of all objects that have been hidden by the script.
2. Unhide only the objects in that list.

The Processed Clients includes Headless Clients

Problem

allPlayers includes headless clients, and they are not filtered out later. Hiding objects for headless clients would be unnecessary and may cause issues with others scripts running on them.

Arma3PerformanceExtension/addons/A3PE/functions/A3PEInit.sqf

Line 55 in b67f011

} forEach allPlayers;

Arma3PerformanceExtension/addons/A3PE/functions/A3PEInit.sqf

Line 73 in b67f011

} forEach allPlayers;

Suggestion

Filter out headless clients by array subtraction (allPlayers - entities "HeadlessClient_F")