Tíquete #44216

Lua: missing destructors

: 2022-03-28 04:11 Última Atualização: 2022-06-24 06:20

Relator:
Dono:
(Nenhum)
Estado:
Aberto
Componente:
Marcos:
Prioridade:
5 - Medium
Gravidade:
5 - Medium
Resolução:
Nenhum
Arquivo:
Nenhum

Details

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.

  • Player:lose(Player winner?, LuaValue loot?) - does mostly the same as killing a king, optionally gives loot (if not ruleset standard one, specified like in game.ruleset:civstyle.gameloss_style) to winner.
  • Player:remove() - works as /remove command
  • City:destroy() - destroys a city. Maybe needs some parameters like what to do with upkept units.

Ticket History (3/11 Histories)

2022-03-28 04:11 Updated by: ihnatus
  • New Ticket "Lua: missing destructors" created
2022-03-28 07:20 Updated by: cazfi
Comentário

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?

2022-03-29 04:18 Updated by: ihnatus
Comentário

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.

2022-03-29 05:38 Updated by: cazfi
Comentário

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.

2022-03-30 07:02 Updated by: ihnatus
Comentário

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.

2022-03-30 17:46 Updated by: cazfi
Comentário

Split city part -> #44229

2022-03-31 04:20 Updated by: None
Comentário

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.

2022-04-15 17:34 Updated by: cazfi
Comentário

Player losing has been split -> #44273

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

Task for actual S3_1d3f: #44342

2022-04-16 03:45 Updated by: ihnatus
Comentário

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

2022-04-23 16:26 Updated by: cazfi
2022-06-24 06:20 Updated by: ihnatus
Comentário

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.

Attachment File List

No attachments

Editar

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login