[Revscriptsys] fixing memory leak in Actions & Moveevents

[EvilHero90]

Pull Request Prelude

• [x] I have followed proper The Forgotten Server code styling.
• [x] I have read and understood the contribution guidelines before making this PR.
• [x] I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

Actions and Moveevents where leaking memory due to having the temporary vectors for registering ids in the pointer itself Since we copy the values when we register the event we also copied the vectors which wheren't cleared already thus creating memory which was not intended. I've moved them to their singletons (Actions/MoveEvents) and clear them every registration

Issues addressed:

• 3694

• 3692

[EPuncker]

related to #3694 and perhaps #3692 (linked on the mentioned PR)?

[MillhioreBT]

It looks good, although these changes make Revscript objects unsafe to store, since they don't contain their own content themselves.

In OTLand I have seen several scripts that create objects at the beginning of the file and register these objects at the end of the file, so each new object will overwrite the vectors and previous objects would be registered incorrectly.

[ranisalt]

It looks good, although these changes make Revscript objects unsafe to store, since they don't contain their own content themselves.

+1 I don't think this is a good solution

[EvilHero90]

It looks good, although these changes make Revscript objects unsafe to store, since they don't contain their own content themselves.

In OTLand I have seen several scripts that create objects at the beginning of the file and register these objects at the end of the file, so each new object will overwrite the vectors and previous objects would be registered incorrectly.

Good catch, however this was actually never intended to work that way anyway, we gave a clear structure on how to create revscripts. I could pass the superior object along with it and if the vectors are not empty but the object is different then throw a warning You are trying to overwrite ids/aids/uids from another Action/Moveevent, please fix your script setup and first register the object before adding ids/aids/uids to the next object

[MillhioreBT]

It looks good, although these changes make Revscript objects unsafe to store, since they don't contain their own content themselves. In OTLand I have seen several scripts that create objects at the beginning of the file and register these objects at the end of the file, so each new object will overwrite the vectors and previous objects would be registered incorrectly.

Good catch, however this was actually never intended to work that way anyway, we gave a clear structure on how to create revscripts. I could pass the superior object along with it and if the vectors are not empty but the object is different then throw a warning You are trying to overwrite ids/aids/uids from another Action/Moveevent, please fix your script setup and first register the object before adding ids/aids/uids to the next object

Lua is well known for its flexibility when it comes to doing what we want with Lua objects, forcing the programmer or user to follow rules and design patterns seems to me to be too much, I know that your revscript 2.0 system solves this , as long as you use your assistant, But if I don't want to use it I will be forced to follow these rules.

I prefer the solution of stealing the vector using swap or simply creating the temporary copy, then cleaning it and then moving it to the actions vector.

example:

or swap:

[EvilHero90]

https://github.com/otland/forgottenserver/pull/4742/commits/6e0a6f3d4c51570c342dbd762abd46b728199301 I've reworked it and made a std::map<Object*, std::vector<uint16_t>> out of it, now it does behave like it was before but we avoid all this copy/borrow/steal stuff @MillhioreBT