Maybe fixes #1342

[tobbi]

No description provided.

[tobbi]

@Vankata453

a) Explain yourself again:

supertux/src/object/moving_sprite.cpp

Lines 106 to 111 in 7ec3cd2

	MovingSprite::has_found_sprite()
	{
	bool found = m_sprite_found;
	m_sprite_found = false; // After the first call, indicate that a custom sprite has not been found.
	return found;
	}

b) Can this have any side effects?

[Vankata453]

1. Spring sprite changing back in editor #1342 has nothing to do with this. It's a hardcoding issue with trampolines that I believe I fixed already some time ago? Not sure.

2. The code you're asking about is kind of a hack for sprite management with object types. It's eeded so when initializing a typed object, it sees whether a custom sprite has been found, and if so, doesn't set the sprite of the type.

[Vankata453]

#1342 should have been fixed on master, does it still change?

[tobbi]

#1342 should have been fixed on master, does it still change?

Yes. Otherwise, I wouldn't have made that PR.

[Vankata453]

Found the issue. It's because Rock calls parse_type() in its constructor, and it is initialized before the derived Trampoline class is. So because parse_type() was called, which calls MovingSprite::on_type_change(), which calls MovingSprite::has_found_sprite(), the m_sprite_found variable has been set to false, which allows the sprite to always be overriden with the default one of the type.

Indeed, the problem is in this function:

MovingSprite::has_found_sprite() 
{ 
  bool found = m_sprite_found; 
  m_sprite_found = false; // After the first call, indicate that a custom sprite has not been found. 
  return found; 
} 

The aim is that it provides whether a custom sprite has been found when constructing the MovingSprite, but also still never returns true on further calls, so changing the object type in the editor would then always override the custom sprite with the default one of the new type.

I am wondering what different solution to have for this problem. One which makes it check if the sprite has been found the first time on_type_change() is called, and for futher calls, doesn't perform the check, so types can always change the sprite. A way it could be achieved is by adding a new variable, which would indicate if an on_type_change() has been performed, but there could be a better solution.

[Vankata453]

Right now I'm trying a potential solution by checking if m_type is smaller than 0, which would indicate it's the first on_type_change() call (or at least, not a call from GameObject::after_editor_set(), from which the protection against changing to default sprite should not run).

[tobbi]

The thing is: pure functions should never change the status of an object and the name of the function (has_found_sprite) suggests that it is a pure function.

[Vankata453]

I agree, it was a very bad hack.

[Vankata453]

Can you try out #2646?

[tobbi]

IDK if your solution is better. But I'm gonna close it in favour of yours.