Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Medical HandleDamageWounds memory leak #6809

Merged
merged 3 commits into from
Mar 31, 2019

Conversation

dedmen
Copy link
Contributor

@dedmen dedmen commented Feb 12, 2019

Do what title says.
compile command caches string indefinitely. Meaning every combination of wounds and pain will stay in memory forever till the game exits or crashes.

@dedmen
Copy link
Contributor Author

dedmen commented Feb 12, 2019

Btw while we're at it.
https://github.com/acemod/ACE3/blob/master/addons/medical/functions/fnc_handleDamage_wounds.sqf#L29 what is up with _woundID? It's never changed, but
https://github.com/acemod/ACE3/blob/master/addons/medical/functions/fnc_handleDamage_wounds.sqf#L62 set onto the unit and causing network traffic.
Actually that variable is never used. It's only initialized to 1 in medical init and then never used again..

Or is woundsOld used if extension isn't there? But why, ACE force kills the mission anyway if the extension isn't available, so that doesn't make much sense.

Ah yeah that's what's happening
https://github.com/acemod/ACE3/blob/master/addons/medical/XEH_preInit.sqf#L11
But. As I said that doesn't make much sense if ACE doesn't let you play anyway if the extension is not there. And if the extension is there we also don't need that variable.
Actually the extension uses that variable. But it's never set back to the lastUniqueWoundID variable.
And the extension itself also doesn't store that it increments the _woundID. Is this a bug?

@PabstMirror PabstMirror added this to the Ongoing milestone Feb 23, 2019
@PabstMirror PabstMirror modified the milestones: Ongoing, Medical Rewrite Mar 25, 2019
@PabstMirror PabstMirror added the kind/cleanup Release Notes: **CHANGED:** label Mar 25, 2019
@PabstMirror
Copy link
Contributor

IIRC, we've talked about dropping unique wound ids

@kymckay
Copy link
Member

kymckay commented Mar 25, 2019

IIRC, we've talked about dropping unique wound ids

Correct, as they would have been used only to do individually targeted treatment which I think we decided is unnecessarily detailed.

Edit: The full conversation happened in slack, but see here #6468

@dedmen
Copy link
Contributor Author

dedmen commented Mar 26, 2019

IIRC, we've talked about dropping unique wound ids

Makes sense, as there currently doesn't seem to be any use nor sense for it. Unless the extension isn't there, which only can happen on linux dedicated server.

@PabstMirror PabstMirror self-assigned this Mar 30, 2019
@PabstMirror PabstMirror changed the base branch from master to medicalExtensionWork March 31, 2019 02:25
@PabstMirror PabstMirror merged commit 3152498 into acemod:medicalExtensionWork Mar 31, 2019
AUTHORS.txt Outdated
Anton
Arcanum417 <lubos.len@gmail.com>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why twice?

jonpas pushed a commit that referenced this pull request Sep 28, 2019
* Fix Medical HandleDamageWounds memory leak (#6809)

* Use strncpy and disable MSVC unsafe warnings (#7171)
* Set 64-bit correctly from generator string, Disable SSE2 flag on 64-bit (non-existent)

* Tweaks for Linux extensions (#5762)
* Tweak CMakeLists for Linux compilation
* Conform SQF extensions check for Linux server extensions support
* Add *.so to tools
* Split extension check into Windows and Linux
* Disable Medical extension loading for now
* Add client/server separation to extension loading
* Add Arma config documentation on extension creation
@PabstMirror PabstMirror modified the milestones: Medical Rewrite, 3.13.0, 3.13.0-temp3 Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Release Notes: **CHANGED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants