Using reassigned game.player.print

[flaghacker_]

I ran into an internal crash on 0.16.21 when playing around with console commands. Steps to reproduce:

• /c print = game.player.print
• /c print("Hello World!")

Results in

factorio-current.log

(8.78 KiB) Downloaded 206 times

[posila]

Hi, thanks for the report.
This is known crash (as in we even use it to test crash handling) and won't be fixed.

It crashes because game.player.print returns temporary object which binds lua and C++ representation of player.print. After the line ends, C++ side of the binding is released and then you call print on released object which results in the crash. So, don't try to store references to API functions in your variables unless you also keep the API object around.

Duplicate: 50221

[quyxkh]

Stuff you type on the console is compiled-on-the-fly into its own new context. The pre-existing context the game supplies is a third layer you can access directly as `_G` (this is a lua thing, not a Factorio thing). `function _G.gpsl(...) game.print(serpent.line(...))` (literal dots) will put the `gpsl` function in that context. There's actually one of those contexts per `control.lua`, i.e. one per mod and one for the map's script, which is a good thing, it means mods don't have to worry about namespace collisions.

What's happening here is, `print` is a lua-supplied function, part of the lua runtime which Factorio augments. It's meant to print to stdout, usually your terminal. So what you did when you said `print=game.print` was to overwrite `_G.print` in the scenario script's context with that temporary reference. The `game.print` reference in `gpsl` above is safe because whats stored is the function, the results of compiling the code to hunt up `game` and `print`, not the results of executing it.

[flaghacker_]

I see, thanks for the explanations!

[Rseding91]

Stuff you type on the console is compiled-on-the-fly into its own new context. The pre-existing context the game supplies is a third layer you can access directly as `_G` (this is a lua thing, not a Factorio thing). `function _G.gpsl(...) game.print(serpent.line(...))` (literal dots) will put the `gpsl` function in that context. There's actually one of those contexts per `control.lua`, i.e. one per mod and one for the map's script, which is a good thing, it means mods don't have to worry about namespace collisions.

What's happening here is, `print` is a lua-supplied function, part of the lua runtime which Factorio augments. It's meant to print to stdout, usually your terminal. So what you did when you said `print=game.print` was to overwrite `_G.print` in the scenario script's context with that temporary reference. The `game.print` reference in `gpsl` above is safe because whats stored is the function, the results of compiling the code to hunt up `game` and `print`, not the results of executing it.

The console is not a new context each time it runs - it's the same context that the scenario script is run in (freeplay if that's what you're running). You can access global variable the same way you do anywhere else: variables not marked as local are global by default.

So:

Code: Select all

/c test = "hello"

Then:

Code: Select all

/c game.print(test)

Will print "hello".

The underlying issue is "v = player.print" returns a lua closure for the function "print" but doesn't capture "player" and then "player" is garbage collected because nothing holds a reference to it and calling "v(...)" tries to call a closure that points at a collected function and crashes.

It would take 2-3 lines of C++ to make that closure also hold a reference to the player object but it would reduce overall performance every time you called anything in the API while not actually fixing the problem that you should have kept player around if you wanted to use it.

[boskid]

This issue is now fixed for 1.1.49. LuaObject method closure will now have the reference to the proxy object in lua state which will prevent the proxy object from being destroyed causing LuaObject from also being destroyed. I was hesitating about this because it does not introduce any new ways of using those closures: they cannot be serialized while they hold the object reference so if they will be kept between events or ticks and a save will happen, they may cause desyncs. Main part is it should no longer crash.