Development Blog

A shared pointer evaluates to NULL if the pointer it contains is gone, but the shared pointer itself isn’t really NULL. I think it uses something like this:

operator==( shared_ptr<T> o )
{
  if (m_valid == false && o == NULL) return true;
  return (get() == o.get());
}

The downside is Lua is somehow storing a shared pointer with an invalid pointer, something that should NEVER happen.

Even if the shared_ptr was being reset properly I think this would affect every single variable in your script program instead of just affecting one variable:

https://github.com/ThePhD/sol2/issues/621

More discussion here:

https://github.com/ThePhD/sol2/issues/129

"This of course may not fit with the programming model you're trying to create, in which case probably the best way to ensure someone can't break your program by trying to do things on deleted objects is to perform additional checks internally (inside your bound methods, etc.) to make sure your shared pointer is still holding a valid pointer."

This is what I was saying before about doing the if statement in all commands on the LE side so things don't crash. Probably a good idea anyway to avoid crashing. However, it sounds like that other guy had a fix in mind but had to go to class. Hope he provides it as it sounds like a bug he's been aware of.

Why would it affect the entire script? That instance of the entire script should be dead at the point of deletion anyway right? I mean internally you're creating an entity object and a script object separately right? Then assigning the script object to a member of the entity? Or is the script object stored inside the entity and then yes deleting the entity should cause the script to be deleted in the destructor of the entity I would think?

Other scripts may reference the entity.

Also there is no way to safely test if a pointer is invalid or if it points to memory that has been reallocated for another object.

Okay so it looks like a shared pointer is being represented by some kind of pointer wrapper, and nullifying the shared pointer right now does not update the pointer value, which is why an invalid pointer was possible. But it looks like it can be fixed and I think your idea will work.

So the issue was that binding library which put a wrapper around a shared pointer?

4 minutes ago, Rick said:

So the issue was that binding library which put a wrapper around a shared pointer?

Well, it is the solution and the problem.

tolua++ does not support shared pointers.

5 minutes ago, Josh said:

tolua++ does not support shared pointers.

Yeah, to get shared pointers working across language domains he had to wrap it, but just forgot about the object that wraps when the shared pointer should be removed. Makes sense. So he's going to fix it?

2 minutes ago, Rick said:

Yeah, to get shared pointers working across language domains he had to wrap it, but just forgot about the object that wraps when the shared pointer should be removed. Makes sense. So he's going to fix it?

Once class is over I guess.

https://github.com/ThePhD/sol2/blob/develop/examples/shared_ptr_modify_in_place.cpp

I will try it out tomorrow. I think this is what will happen::

local model = CreateBox()
Reset(model)
print(model==nil) --prints false, but object disappears immediately
model = nil --shared_ptr is set to nil
print(model==nil) --prints true

local model = CreateBox()
Reset(model)
if model~=nil then
	model:SetPosition(1,2,3) --ERROR: userdata does not have a function called SetPosition
end

 

Maybe create a Delete() lua function that both calls the c++ Reset() and sets that obj to nil? Kills 2 birds with 1 stone then and is easy for the user. Not sure if there is a use case for calling Reset() without setting the lua side to nil. I wouldn’t think so.

3 hours ago, Rick said:

Maybe create a Delete() lua function that both calls the c++ Reset() and sets that obj to nil? Kills 2 birds with 1 stone then and is easy for the user. Not sure if there is a use case for calling Reset() without setting the lua side to nil. I wouldn’t think so.

Is that even possible? I’m not sure.

I'm saying create this function in Main.lua but it would be cool if from C++ one could actually set the obj passed into the function to nil so that Lua will see it as nil. You'd have to think that's possible some way which would be nice.

function Delete(obj)
  Reset(obj)
  obj = nil
end

 

It's actually worse than that. See my response here:
https://github.com/ThePhD/sol2/issues/129

This is why I mentioned the Exists() c++ function. When you reset the shared pointer you’re doing this on the c++ side of things. So you’ll probably need to check on the c++ side of things if it’s valid. So our checks on lua would be to pass our reference to the Exists() to see if that shared pointer is still valid or not. The other approach is you checking if it’s valid in all calls of using it which is probably the way to go to prevent crashes but more work for you. Not sure if that creates a side effect of the user wondering why calls aren’t doing anything so still having the Exists() function is recommended. Think of the Exists(obj) function as the if == nil check. It gives us a way to validate if the shared pointer is valid or not on objects that may be shared between scripts. A feature that in LE 4 we don't have so that's a win right there! Remember the case of the Tower Defense where 2 towers have 1 enemy unit as a target but one kills it before the other but then the other doesn't know this and tries to kill it too and the game crashes currently. We have to do workarounds to avoid that situation. With the above idea we wouldn't have to. We'd simply check if Exists(target) then.

We'd only really ever need to use the Exists() function if we know the object is shared between scripts. That doesn't happen on every object we create so it's not as if we need to wrap everything in an Exists() check. 99% of the time we'd just call Reset(obj) obj = nil and be done with it. (Not a fan of the Reset() name though as it doesn't really give a good description of what it's doing from a users standpoint).

I just don’t get how deleting something and expecting it to be gone is a bad idea and against user expectations. If I delete something I expect it gone.Thats the entire point of deletion and it’s how le4 works, and any other engine works so that concept itself isn’t against user expectations.

I don’t think it’s just visual objects to right? Wouldn’t sounds have the same requirement of stopping before deletion?

 

I think in practice we will find there is always one piece of code that “owns” the object and it’s okay to hide it or stop a sound that is playing. Texture usage, on the other hand, is something where you never really know how many instances are active but that is handled internally by the engine. Smart pointers make the internals of Leadwerks MUCH simpler. Without smart pointers I could not write the new editor in C++.

The whole point is to relieve the user from the manual reference counting system which few people understand. I always fell bad explaining that part to beginners and I suspect mistakes made with invalid pointers have been an issue in some of the larger projects people have made with Leadwerks.

The real benefit you're getting though is the ability to do an if check on the smart pointer to see if it really exists or not because everything created is shared between the users code and the internal engine and you want to avoid crashes like what happens today in LE4. Smart pointers are doing great at that but they now have this side effect of keeping things around when us users don't want them around anymore.

I don't get why having a Delete() and Exists()/IsValid()/whatever is a problem to fix the issue. Seems fairly logical doesn't it? Just trying to better understand why you and the other github guy don't think so unless he doesn't understand the context in which you're using his library since most things aren't as visual as a game engine where it probably doesn't matter if the thing hangs around.

Setting to nil and calling collectgarbage() totally eliminates this. I really don’t like the idea of explaining to users, “well the object isn’t null but it also doesn’t exist.” Lua should be simpler than C++, not more complicated.

11 minutes ago, Josh said:

Setting to nil and calling collectgarbage() totally eliminates this. I really don’t like the idea of explaining to users, “well the object isn’t null but it also doesn’t exist.” Lua should be simpler than C++, not more complicated.

In the script where you set the variable to nil you don't have to do an Exists() check just a simple nil check because in that script that variable is already nil. This only happens in the situation where the user shared the object between scripts. That's the point where you have to check if the object still exists or not because any of the scripts that may have access to it could have deleted it. Is that not a simple concept? It's something we wish we had in LE4 and have talked about getting it before but there was no solution this simple because of the way pointers were handled. Shared pointers gives that simple solution!

Yet I don't think setting to nil and calling collectgarbage() does solve the issue. If I have a model object shared between 2 scripts and one sets it's variable to it to nil and calls collectgarbage() does the other one not still have it because it did a copy operation when it assigned it to it's local variable. That would have created 2 counts of the shared pointer so it would still stay alive would it not? Think about the tower defense situation. One tower thought it killed the enemy unit but it really didn't because the other tower still had a reference to it so it stays alive. Doesn't sound like the desired behavior in that situation or really any situation where this can happen in a game.

If the fear is a user forgets to call Exists(obj) and the game crashes (again you should have checks in all calls I think) then that same fear exists today and we've dealt with it for a long time. At least with the Exists() function we have a way to deal with it that doesn't require jumping through hoops we have to do now to get this desired behavior.

I personally think explaining garbage collecting to users is more complicated given that in the scenario I posted above the garbage collecting wouldn't actually delete the object because another script had a reference to it.

This is how I would handle it.

Script 1:

enemy.health = enemy.health - 1
if enemy.health <= 0 then
  enemy:Hide()
  enemy = nil
end

Tower script:

if self.target ~= nil then
  if self.target.health < 1 then
    self.target = nil
  else
    -- update the AI code
  end
end

 

-- script 1
enemy.health = enemy.health - 1
if enemy.health <= 0 then
	Destroy(enemy)
    enemy = nil
end


-- script 2
if Exists(self.target) then
	self.target:Hurt(50)
end

This is how I would want to handle it.  Exists() is a C++ function checking the shared pointer if it's valid or not. I have my opinions on which one would be easier to explain to a user.

At this point I think my opinion is out there so I'll stop on this specific point.

There are some other issues. I believe it would behave like this:

local a = CreateBox(world,1,1,1)
local b = a --same entity, same shared_ptr

local t = world:GetEntitiesInAABB(-10,10,-10,10,-10,10)
local c = t[1] --same entity, different shared_ptr

Reset(a)

print(Exists(a)) --false
print(Exists(b)) --false
print(Exists(c)) --true

This also requires a wrapper function for every command that receives a shared_ptr as an argument. We are trying to improve the binding code to eliminate wrapper functions. If I make this a requirement, it is an added constraint. Will I be happy with that decision in three years? Will be who are trying to bind their own commands be happy with this? Will having this added memory management mode cause problems in the interfacing between different editor extensions or entity scripts? I don't know.

Yes, that would be an issue. I would expect there is 1 shared pointer per actual pointer but if it doesn't work that way then this wouldn't work. The question is does that cause other issues too? It will be interesting to see if posts happen around this entire shared pointer idea on the lua side.

Example showing 2 shared pointers pointing to the same underlying pointer and how reset on 1 shared pointer doesn't affect the other.

http://cpp.sh/7y4sp