Freeciv

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.

Player losing has been split -> #44273

That leaves only the Player:remove() to this ticket.

Task for actual S3_1d3f: #44342

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?..)