fix: entity removal for excluded entities (disconnect, respawn)#86
fix: entity removal for excluded entities (disconnect, respawn)#86
Conversation
The dedup optimization only sends state updates when data changes, but excluded entities (disconnected players, respawn corpses) stop producing states abruptly — the extension's gap-fill has no final state to extend. - Disconnect: stop excluding the unit body so it continues being tracked as AI. The capture loop's reconnect logic handles re-inclusion. - Respawn: send a final lifeState=0 state for the corpse before excluding it, so the extension knows the corpse's last frame for gap-filling. Companion to OCAP2/extension#134.
Guard against potential script error if unitData is unexpectedly a non-array type by checking isEqualType before using the + copy operator.
Restores exclude=true so disconnected player bodies stop being tracked, but first sends a final lifeState=0 state so the extension knows the entity's last frame. Combined with the extension's dead-entity handling, the unit properly disappears from playback at the disconnect frame.
Send :SOLDIER:DELETE: on disconnect and respawn instead of injecting a fake dead state. The extension now owns the gap-fill boundary logic.
respawnOnStart reuses the same unit object, so _entity == _corpse in the EntityRespawned handler. Sending DELETE in this case caused the entity to be prematurely removed in streaming mode while the capture loop re-included it on the next frame.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the entity tracking and removal mechanisms within the recording system, particularly for player disconnects and respawns. It ensures that entities are correctly marked for deletion by sending explicit commands, while also preventing unintended removals in specific respawn scenarios. Additionally, it includes minor configuration adjustments and enhances logging for better operational visibility. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly implements entity deletion for disconnected players and respawned corpses by sending a :SOLDIER:DELETE: command, replacing the previous method of faking a dead state. The changes also properly handle the respawnOnStart scenario and add some useful logging for saving recordings and settings snapshots. My review includes suggestions to improve the robustness of the new entity deletion logic by adding checks for invalid IDs before sending data to the extension.
| [":SOLDIER:DELETE:", [ | ||
| _corpse getVariable [QGVARMAIN(id), -1], | ||
| GVAR(captureFrameNo) | ||
| ]] call EFUNC(extension,sendData); | ||
| _corpse setVariable [QGVARMAIN(exclude), true]; |
There was a problem hiding this comment.
The code retrieves the corpse's ID using _corpse getVariable [QGVARMAIN(id), -1], which defaults to -1 if the ID is not found. A :SOLDIER:DELETE: command is then sent with this ID. While the isInitialized check on line 76 should prevent this, other parts of the codebase, like fnc_eh_killed.sqf, explicitly check for an ID of -1 before processing. It would be more robust to add a similar check here to prevent sending delete commands for an invalid ID.
private _corpseId = _corpse getVariable [QGVARMAIN(id), -1];
if (_corpseId != -1) then {
[":SOLDIER:DELETE:", [
_corpseId,
GVAR(captureFrameNo)
]] call EFUNC(extension,sendData);
};
_corpse setVariable [QGVARMAIN(exclude), true];
| [":SOLDIER:DELETE:", [ | ||
| _unit getVariable [QGVARMAIN(id), -1], | ||
| GVAR(captureFrameNo) | ||
| ]] call EFUNC(extension,sendData); | ||
| _unit setVariable [QGVARMAIN(exclude), true]; |
There was a problem hiding this comment.
Similar to the change in fnc_addEventMission.sqf, this code sends a :SOLDIER:DELETE: command using an ID that defaults to -1. For robustness and consistency with other parts of the code like fnc_eh_killed.sqf, it's better to check if the ID is valid (not -1) before sending the command.
private _unitId = _unit getVariable [QGVARMAIN(id), -1];
if (_unitId != -1) then {
[":SOLDIER:DELETE:", [
_unitId,
GVAR(captureFrameNo)
]] call EFUNC(extension,sendData);
};
_unit setVariable [QGVARMAIN(exclude), true];
Summary
:SOLDIER:DELETE:on disconnect and respawn (corpse) instead of faking a dead state_entity == _corpse(respawnOnStart reuses the same unit object) to prevent premature removal in streaming mode:SOLDIER:DELETE:/:VEHICLE:DELETE:commands)Changes
:SOLDIER:DELETE:+ exclude instead of injecting fake dead state:SOLDIER:DELETE:for corpse, guarded by_entity != _corpseto handle respawnOnStartTest plan
hemtt buildcompiles