multitheftauto / mtasa-blue

Multi Theft Auto is a game engine that incorporates an extendable network play element into a proprietary commercial single-player game.
https://multitheftauto.com
GNU General Public License v3.0
1.41k stars 438 forks source link

triggerEvent/addEvent issue #2448

Closed flashmta closed 2 years ago

flashmta commented 2 years ago

Describe the bug

triggerEvent is not working when a given resource start/stop order is used.

To reproduce

  1. start test3
  2. start test2
  3. stop test3
  4. start test1

test.zip

Expected behaviour

The event should be triggered (message should appear in debug console)

Version

Multi Theft Auto v1.5.9-release-21024.1

Lpsd commented 2 years ago

This is because you originally add the event in test3. When an event is added for the first time, that resource essentially has ownership of the event (it's "tied" to the Lua VM of that resource).

Since your event is already registered and owned by test3, the addEvent call within test2 resource has no effect (the addEventHandler call still works fine as expected)

When test3 is stopped, since it has ownership of the event, that event is removed completely - regardless of if other resources have handlers for the event. In this case, it doesn't matter that test2 has a handler for your event, since the event doesn't exist anymore.

I do think that ownership of events should be cascaded down to other resources when the owner resource has been stopped. It should be done in the order of the event handlers being added (oldest -> newest).

In this instance, test2 would take ownership of your event when test3 stops, since it was the next resource (in order) to handle the event.

A small concern is whether we have to worry about backwards compatibility for this (regardless of our stance on bwc. for 1.6) - people may expect events to be destroyed when their resource owner is stopped. Or rather, should there be a parameter to choose the behaviour?

Pieter-Dewachter commented 2 years ago

I have made a PR that resolves this issue a while ago, perhaps it's a good time to give it a new review? https://github.com/multitheftauto/mtasa-blue/pull/2333

Lpsd commented 2 years ago

Closed by #2333