-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
RFC: Safe LuaObject:ApplyAt API #5222
base: master
Are you sure you want to change the base?
Conversation
This is an RFC API aiming to mitigate the safety problems caused by Timer:CallAt as seen in plenty of issues, the latest of which is pioneerspacesim#5220. The fundamental issue with the CallAt API is that the environment in which the closure is called can be drastically different to the one in which it was defined. This new API aims to guarantee the caller that at least one object must still be valid when calling the closure, by storing it out of band and having the engine check its validity before calling the callback. This would allow some simplification in existing correct callbacks, e.g. in the assassination module, we could turn this Timer:CallAt(mission.due, function () if mission.ship:exists() then mission.ship:Undock() into mission.ship:CallAt(mission.due, Ship.Undock) Thoughts ?
7ba0033
to
1c02354
Compare
@laarmen what if the second argument is a method not from the 'Ship' table, but from another table (Space, for example)? Will the object be passed there as self? |
I think so. My Lua memories are a bit fuzzy, but IIRC the whole "method" syntax is purely syntactical sugar. When I wrote the example, I wasn't planning on it being reduced to directly passing a method, it was just a happy coincidence. You could also write it as follows. mission.ship:CallAt(mission.due, function (s) s:Undock() end) edit: fix syntax method in the code block. |
@laarmen
imho in that case it will even be easy to serialize, although it only becomes usable for this object. |
Serialization is out of scope. This API is meant to reduce the amount of boilerplate necessary for delayed calls in the Lua scripts. Such calls are not currently serialized (unless I'm missing something?). I am vehemently against using strings instead of actual functions. See https://devcards.io/stringly-typed for more details on this. If you want to ensure you're using the correct method for your object, you can simply write mission.ship:CallAt(mission.due, mission.ship.Undock)
-- or
mission.chip:CallAt(mission.due, function(s) s:Undock() end) |
@laarmen but lua is not strongly typed, and |
I like this idea, though I think on its own it merely reduces to If we're going to go the route of adding additional API specifically for this type of pattern, I think we should look more closely at LuaEvent as well - it's a common pattern to write a function preamble in event handlers that tests if a specific argument a) still exists, and/or b) is a specific object or class of objects. This seems like a similar requirement to that proposed for Timer, and if an API pattern can be found that covers both use-cases it would certainly improve the code quality significantly. |
876b6fc
to
b5bc47c
Compare
90e4530
to
6a3a044
Compare
This is an RFC API aiming to mitigate the safety problems caused by
Timer:CallAt as seen in plenty of issues, the latest of which is #5220.
The fundamental issue with the CallAt API is that the environment in
which the closure is called can be drastically different to the one in
which it was defined.
This new API aims to guarantee the caller that at least one object must
still be valid when calling the closure, by storing it out of band and
having the engine check its validity before calling the callback.
This would allow some simplification in existing correct callbacks, e.g.
in the assassination module, we could turn this
into
Thoughts ?