MSRevive / MasterSwordRebirth

Continuation of Master Sword Classic/Continued.
https://msrebirth.net/
Other
9 stars 6 forks source link

Memory leak with client-side scripts #31

Closed greatguys1 closed 2 years ago

greatguys1 commented 2 years ago

When a script uses the script command "clientevent new <player|all> [params...]" , the space allocated to use that command isn't cleared when the script that was created from that command uses the script command "removescript".

I made a test script that demonstrates this. In the console, type: dev_on wargwable . createnpc developer/rex/effect_leak

Then say "effect" to the rat in the npc chat. Using Task manager you can see the memory usage climb. It is creating scripts on the client side, then they're deleting themselves, but the memory isn't being cleared. You can say "off" to the rat to get it to stop.

"clientevent new", the command that creates the client side script, CScript::ScriptCmd_ClientEvent

The client receives the message at CHudScript::MsgFunc_ClientScript

"removescript", the command that should remove the client side script, CScript::ScriptCmd_RemoveScript

Attached is the script made to test the leak and the compiled form of it sc.zip .

greatguys1 commented 2 years ago

I think I found a fix

In IScripted::RunScriptEvents delete[] Script should be delete Script

Can someone verify?

SaintWish commented 2 years ago

I think I found a fix

In IScripted::RunScriptEvents delete[] Script should be delete Script

Can someone verify?

I wasn't sure if you were correct since according to this https://stackoverflow.com/questions/2425728/delete-vs-delete-operators-in-c I think it should be delete[], but I tried it with just delete like you said and I experienced no memory leak... I'll commit the fix and send you the DLLs in DMs.

EDIT: I figured why it's like that it's because it's set to single object not an array of objects...