-
Notifications
You must be signed in to change notification settings - Fork 398
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
Lua connect and disconnect event handler bug #827
Comments
I am currently trying to reproduce this on latest commit Adding the following snippet at the end of GameInterface.init makes it occur:
|
In order to fix this I would need to change LuaInterface::signalCall function to also call base classes event handlers or LuaInterface::registerClass, so that base class knows all classes that derive from it (so it can install event handlers in derived classes too). |
Creature can be: Connecting Creature.onPositionChange will be triggered for any of the above cases, it is correct that it will be called for any creature movement. But LocalPlayer.onPositionChange should be called just for LocalPlayer Eryn is another player in another client ? |
Eryn is actually an NPC in tfs |
It is a bug. It happens because of __index method within metatable of all derived classes which fallbacks to the base class and thus gets the handler from it. I did take a look at your fix and it seems to be quite extensive. However, considering the fact that it appears to work in reversed order (create a handler within derived class and then a next handler within base class) then I don't quite understand why it won't work with rawget and rawset instead. Anyway, I should be able to check the issue more closely tomorrow, so stay tuned. Meanwhile I will summon two guys: |
hasValue could be rewritten to be using rawget, but I don't see any point in it. |
Indeed it's a bug, the fix could be much simplified and cleaner by using a |
If I understand the issue correctly only the hasValue(object, value) can be replaced with rawget. |
He means replacing these two: https://github.com/edubart/otclient/blob/master/modules/corelib/util.lua#L56 if not object[signal] then => if not rawget(object, signal) then I thought about the same, but I wasn't sure whether it will be enough. I didn't have time to test it yet. |
I don't think that these two changes are enough, because then base class event handlers would be skipped (if event happened for a player and had some handlers there, the event handlers for creature wouldn't be called) |
Ah, I believe I do understand you now. You mean if local player triggers onPositionChange it should fire all handlers from both Creature and LocalPlayer, but instead it will only fire the top ones within derived class (LocalPlayer) leaving those from Creature. However, since local player is also a creature it should fire them all. |
Exactly, that's why I added createHandler |
Seems that issue is solved in #835 which already has an open PR. Does anyone tested it? Is it ready to merge this PR? |
@danilopucci I don't think this project has a maintainer anymore, would you mind talking to edubart and see if he could let you be one of the maintainers, or even better, try to switch to otland fork instead? https://github.com/otland/otclient there at least we have the staff constantly reviewing |
Hello.
There is a bug that sometimes makes handlers for LocalPlayer trigger for Creature events etc.
I.e. in a (not so) special case any handler bound to a derived class can be triggered by base class event.
Steps to reproduce:
bind lua handler to Creature.onPositionChange that does nothing
bind lua handler to LocalPlayer.onPositionChange(player) that prints player:getName()
Expected behaviour:
The only prints in console are local player's name.
Actual behaviour:
When any creature moves, it's name appears in console
The reason behind this is that connect function searches for list of signal handlers in any of base classes and if it hasn't found any, it creates one in current class.
The call to bind handler to LocalPlayer finds one in Creature (that is base class of LocalPlayer) and inserts event handler there, so calls by Creature events are also triggering LocalPlayer handlers.
The fix would be using rawget and rawset to always add trigger handler in current object, and in signalcall to iterate over metatables in order to trigger handlers of all base classes.
Please confirm that this is actually a bug, and not a feature.
The text was updated successfully, but these errors were encountered: