we shouldn't use ipairs to iterate over entities

We can define our own iterator function like in

function ourpairs(tab)
   local i = 0
   return function ()
      if i <= #tab then
         local result = tab[i]
         i = i + 1
         return i - 1, result
      else
         return nil
      end
   end
end

This creates a closure, which is not neccessary, but you get the idea.

My proposal is to have a GentityRef pointing to the current juggernaut

Why are we even talking about fringe mods here?

This creates a closure, which is not neccessary, but you get the idea.

The proper way to do this would be

function ournext(t, i)
    i = i + 1
    local v = t[i]
    if v ~= nil then
        return i, v
    end
end

function ourpairs(tab)
   return ournext, tab, -1
end

What is now needed to merge this?

a decision on the API

a decision on the API

Is it a decision you can make?

At some point we have to merge something, or this will never be.

Is it a decision you can make?

That would be a nasty descision. There is a very real problem here that needs to be resolved somehow. I am not quite sure whether any of the proposed solutions are adequate. One proposed method is a poor person's generational garbage collector, and another is to accept that we will be running into subtle bugs randomly.

I would like to propose a different API, reminiscent of Unix system calls. Unix became popular by simply not solving such complex problems in the kernel, but by delegating the burden to the user space. Its system calls usually work fine, but a caller has to be prepared for the occasional mysterious failure. We could do something similar with our Lua API, thereby keeping things simple.

We could do something similar with our Lua API, thereby keeping things simple.

I give an example in the context of map entities in #3330 (comment). The method described there does not use proxy objects, so the API does not have to worry about whether they are still valid. Instead, it relies on callbacks. These callbacks can handle the problematic cases themselves. Thus, the API itself is very simple.

Entity reference invalidation is not a complex problem. It is a very easy problem. All you have to do is store the generation number (no relation to generational garbage collection) on the proxy object, and then when then a reference to a freed entity whose slot was recycled is used, treat it the same as if a reference to a freed entity whose slot is empty were used.

Entity reference invalidation is not a complex problem. It is a very easy problem.

But we will still need to guard each occurance of entity[key] in the code. There is no way around this with the API proposed here. That will be just as noisy as the other method I proposed.

But we will still need to guard each occurance of entity[key] in the code.

What do you mean, in Lua scripts or C++ code? In Lua code you are no more or less obligated to check your references for validity than before. References can already be invalidated when the entity is freed. On the C++ side, yes the references need to be checked for validity. They already do anyway: we must check inuse before using reference. This is just a better check that also detects invalid references to slots that have been reused.

EDIT: Now I think I understand the point of that comment. I was comparing against the current state of this PR #3064, while Sweet is comparing against a proposal in another thread. Yes, that's a valid point that you might need to check the validity of the entity each time you use it from Lua. It's true that event-driven programming without entity references can be more ergonomic for some use cases. But we still need entity references because people are going to want them (and resort to raw entity nums if they're not available) for other use cases. An example of this type of logic is the hive's and rocketpod's smart missiles. They need to keep track of the current target and each frame, check if the target is still valid and if so, steer toward the target. This would be much more awkward if we had to register event handlers for all the ways the target can become invalid (death, changing team, disconnecting, not sure if others?).

Sweet is comparing against a proposal in another thread

Yes. I should have been more clear.

This would be much more awkward if we had to register event handlers for all the ways the target can become invalid (death, changing team, disconnecting, not sure if others?).

I guess you are right. Such event handlers would be very messy indeed. They would be even more messy if they avoided allocating lua functions at runtime, which is probably a requirement for such cases.

I am in favour of the generation counter now, be it GentityRef or something else.

Also note that in my initial attempt, I did try to use a gentityref, it was just super unergonomic so I had to revert it.

02a89ed

Hey, look, the commit is still around :)

If there is consensus that gentifyref is better, then we can consolidate around that. I think we can just establish a rule to not maintain references to entities in callbacks, only track the entity numbers and that those entities may become invalid so you also must track those with different event handlers.

I'm not sure that there is any way around this if you want a persistent way to track a player.

I think we can just establish a rule to not maintain references to entities in callbacks, only track the entity numbers and that those entities may become invalid so you also must track those with different event handlers.

Did you mean s/entity/client/ here?

I'm not sure that there is any way around this if you want a persistent way to track a player.

Maybe assigning IDs to players could be useful in some cases. Though I'm not sure it would be any easier than client num + event handlers. We could also make a ClientRef for this. But then we might need two variants of ClientRef, one that invalidates on leaving the team and one that invalidates on disconnect?

Did you mean s/entity/client/ here?

I didn't, but I realize I'm wrong.

Consider my shitty dretch tutorial: https://github.com/DolceTriade/unvanquished-tutorial/blob/main/game/tutorials/dretch.lua#L14

I maintain a reference to the egg. I should not do this. I should give that entity an ID and every time I want to reference it, I need to re-fetch it using that ID (unless it is already passed in for me using a callback).

I cannot use the entity number because there is no guarantee that the entity number won't belong to a different egg in the future if I forgot to track its death via its die handler.

Maybe assigning IDs to players could be useful in some cases. Though I'm not sure it would be any easier than client num + event handlers. We could also make a ClientRef for this. But then we might need two variants of ClientRef, one that invalidates on leaving the team and one that invalidates on disconnect?

Eh, I think for clients clientNum or manually assigning an ID is sufficient. Manually Assigning an ID is probably better actually because then I don't need to worry about disconnection (though I think I need to worry about team change?) because the ID shouldn't change?

Also I'm not fully clear on the client entity life cycle, because the server maintains some state about disconnected clients and I haven't dug into this code to see how it works.

In any case, I think gentityRef is fine with either clientNum or manual IDs.

I should give that entity an ID and every time I want to reference it, I need to re-fetch it using that ID (unless it is already passed in for me using a callback).

It is currently unclear if you should really use an ID to reference anything that is not a map entity. There is a (questionable) PR to assign IDs to the reactor and the overmind. But if this makes some sense, then only because these buildings are unique.

The main idea about entity IDs was that a mapper may provide them. If the mappers does not, we can provide human readable IDs for them.

As far as I know, we do not do any defragmentation in the entity array. An entity number will be valid until the entity is removed. So there is no real advantage in using an ID over the index. The best automatic IDs we could do for eggs would probably look like egg_18473. Why would that be useful? If the only use is to catch the case of a vanishing entity, we can probably do better.

That being said, my proposal in #3345 (comment) could be used in the way you are describing.

I should give that entity an ID and every time I want to reference it, I need to re-fetch it using that ID

There is a proposal for the ID mechnism itself hidden in thise sentence. Currently, we require that there are not two entities with the same ID at the same time. You are proposing to strenghten this requirement such that two entities must not have the same the ID in all of time.

One motivation for the ID field was to have something similar to the HTML DOM attribute id. In HTML, it is perfectly acceptable to have several nodes share an ID, as long as this does not happen at the same time.

There is a proposal for the ID mechnism itself hidden in thise sentence. Currently, we require that there are not two entities with the same ID at the same time. You are proposing to strenghten this requirement such that two entities must not have the same the ID in all of time.

I would be in favor of following this principle for automatic IDs.

It is currently unclear if you should really use an ID to reference anything that is not a map entity.

Assuming we have GentityRef, then indeed probably most of the entity handles in your scripts should be GentityRef-based entity objects, not IDs. But I can think of a few reasons you might want to use user-assigned IDs:

• Making handles for clients that last until disconnection. (Client entities exist until disconnection but ClientSpawn bumps the generation counter.)
• Debugging/visibility for the user. Make it easy to identity entities in entitylist etc. or do commands on them.
• Inter-script interaction. Someday we will probably want to implement separate global variable namespaces for separate Lua scripts so they don't step on each other. In that case IDs could be useful to find an entity created or tagged by another script.

The best automatic IDs we could do for eggs would probably look like egg_18473. Why would that be useful?

I don't think anyone is proposing that. Ishq was saying how manually-assigned IDs could be used as a poor man's GentityRef in the context of a dretch tutorial that needs to keep a persistent reference to a particular egg.

I want to maintain that behavior. Let me give a concrete example...

I build an egg using Lua (or adopt an en existing egg in the map, that's not important). I want to make sure that I am always referencing this egg and not another egg.

With the current implementation of this PR, I can do something like this:

MYGLOBALEGG = nil

function makeEgg()
  local egg = sgame.SpawnBuildable('eggpod', node.origin, node.angles, node.origin2, true)
  egg.die = function()
    MYGLOBALEGG = nil
  end
  MYGLOBALEGG = egg
end

function whereEgg()
  if MYGLOBALEGG == nil then
    return
  end
  print("My egg is alive!!!")
  -- Keep checking egg every second
  Timer.add(1000, whereEgg)
end


makeEgg()
-- Check egg every second
Timer.add(1000, whereEgg)

I can store my reference to my egg outside of the scope and it will forever point to that entity number. Even if it dies or changes or anything else. So I must register a death handler to notify me when it changes (as death is the only way this particular entity can change).

Technically speaking, even if I migrate to entityrefs, the above code will continue to be valid because eggs don't evolve, so as I am typing this, this is probably a bad example, but whatever, I'm continuing forward...

function makeEgg()
  local egg = sgame.SpawnBuildable('eggpod', node.origin, node.angles, node.origin2, true)
  egg.id = "specialegg"
end

function whereEgg()
  local egg = sgame.entities.find_entity_by_id("specialegg")
  if egg == nil then
    return
  end
  print("My egg is alive!!!")
  -- Keep checking egg every second
  Timer.add(1000, whereEgg)
end


makeEgg()
-- Check egg every second
Timer.add(1000, whereEgg)

No references to entities outside the scope from which they were gotten. Since the ID is freed when an entity is destroyed, after the entity is gone, I won't be able to find it anymore.

Technically, ANOTHER lua script could have run and given another egg the same ID, but I'm discounting that case because these IDs must be given explicitly and any other lua scripts must also be run explicitly and I don't think I worry about that case in practice.

A potentially problematic implementation might be...

egg_id = nil

function makeEgg()
  local egg = sgame.SpawnBuildable('eggpod', node.origin, node.angles, node.origin2, true)
  egg_id = egg.number
end

function whereEgg()
  local egg = sgame.entities[egg.number]
  if egg == nil then
    return
  end
  print("My egg is alive!!!")
  -- Keep checking egg every second
  Timer.add(1000, whereEgg)
end


makeEgg()
-- Check egg every second
Timer.add(1000, whereEgg)

In this case, even if the egg died, another entity could have taken over its number so in this case, I would need to register a death handler for this code to be correct.

After typing all this out, I learned that entityRefs literally only affect things for clients and clients require special handling no matter what (team change, disconnect, death, evolution, weapon buys (no entity impact for this one), admin level changes, etc), so you probably need to constantly validate your client no matter what approach we take. I think we cannot get around this.

Notice that #3330 (comment) solves the nasty problem that has stalled this PR for many months.

I honestly do not get why you two are making this so complicated.