Lua: missing destructors
Destroying things from the lua script just has the problem (probably present with already existing destructive methods, though we've tried to fix code where ever possible) that they may destroy things where the C-code is not prepared for it. Are we ready to open that can of worms as about the last thing before going to freeze?
Reply To cazfi
Destroying things from the lua script just has the problem (probably present with already existing destructive methods, though we've tried to fix code where ever possible) that they may destroy things where the C-code is not prepared for it. Are we ready to open that can of worms as about the last thing before going to freeze?
As I can remember, 99% of callbacks already do have checks of objects involved. Verifying all of them is necessary any way since e.g. a city may already be destroyed indirectly by a unit action, and it can be done after d3f.
Reply To ihnatus
a city may already be destroyed indirectly by a unit action
That's a very valid point about the city removal method.
I don't think I've ever read through the player *removal* code, so won't say anything too final about that part yet. I know that we've never had player's *death* handled (invalidating any data structures, iterators) mid-turn, but at most flagging it in 'is_alive' to be handled in the next turn change.
Reply To cazfi
Reply To ihnatus
a city may already be destroyed indirectly by a unit action
That's a very valid point about the city removal method. I don't think I've ever read through the player *removal* code, so won't say anything too final about that part yet. I know that we've never had player's *death* handled (invalidating any data structures, iterators) mid-turn, but at most flagging it in 'is_alive' to be handled in the next turn change.
Player may be set to dying state in a callback the same way as a city may be destroyed, just a unit does an action and the king falls... I have done checks for players in the loops in my latest patch for srv_main.c, but well, we should test another places. There is too many places when we indirectly invoke "unit_move" to test them all fast, yup. But it should be done.
Split city part -> #44229
Reply To ihnatus
As I can remember, 99% of callbacks already do have checks of objects involved
Heck, read the code... hardly there is a single check of anything in 50% of script_server_signal_emit invocations within the very inner block before using the variables again... Yes, tons of work insue, but we can't build a stable application without it.
Probably we can postpone removing method to 3.2. While it's fine to have a destructor for what is destroyable, removing players is not what we do too often. I think some campaign-like scenario may need things like this but actually even there it's not a 100% necessity comparing to other present problems.
(Side note: what's there with reviving a player? A dead (is_alive == FALSE) player is revived in editor if you create a unit or a city of that nation, but doesn't a script create a zombie nation now?..)
I think we can leave removing player data on the next kill_dying_players() call, just setting some another state or a special switch. There is no need for an immediate action that would require tons of code complications.
3.2 going to d3f as soon as possible. Postponing all the remaining blockers that can be postponed.
Since #42501 is going to be postponed to 3.2, I request separately a thing that should for better go to S3_1: ways to destroy/inactivate game objects.