Can't flip sprite

Started by jumpjack, Tue 03/01/2023 16:40:32

Previous topic - Next topic

jumpjack

What's wrong with this code?

Code: ags
      DynamicSprite* flipped = DynamicSprite.CreateFromExistingSprite(tileIdNorm,  true); 
      flipped.Flip(eFlipLeftToRight); 
      roomOverlay[overlayIndex] = Overlay.CreateRoomGraphical(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic);
      roomOverlay[overlayIndex].ZOrder = isoX + isoY;
      background.DrawImage(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic);

drawImage properly draws the flipped sprite, but CreateRoomGraphical draws sprite slot n.0, although flipped.Graphic is = 65 , and I have static sprites numbered from 0 to 64.

Documentation:
Code: ags

static Overlay* Overlay.CreateRoomGraphical(int x, int y, int slot, optional bool transparent, optional bool clone)

DrawingSurface.DrawImage(int x, int y, int slot, optional int transparency,
                            optional int width, optional int height,
                            optional int cut_x, optional  int cut_y, 
                            optional int cut_width, optional int cut_height)

Khris

If the first line is inside the function then flipped ceases to exist when the function finishes.
It's news to me that AGS keeps a reference to the original sprite and falls back to it but maybe that's how it works internally.

Anyway, the solution is to add
Code: ags
DynamicSprite* flipped;
above the function, then do
Code: ags
  flipped = DynamicSprite.CreateFromExistingSprite(tileIdNorm,  true);
inside.

Crimson Wizard

#2
When you assign DynamicSprite to an object, it normally is not copied, but referenced by the sprite ID (this is their Graphic property). Like Khris mentioned above, in AGS script dynamic sprites, like overlays, are deleted when the last pointer to them is removed, which happens when the function ends in your case. This is similar to Overlays. As soon as dynamic sprite is destroyed, the game object looses it and switches to sprite 0, which is historically used as a "placeholder".

Painting the dynamic sprite onto background will stay, as it changes background image, so it no longer depends on a sprite's presence.

But in regards to Overlays, there's another opportunity: CreateOverlay* functions have an optional "clone" argument. It's default value is "false", but if you pass "true", then they will make a copy of an assigned sprite, and the original sprite may get safely deleted after. So, if you do not want to keep the sprite afterwards, you may use this method.

Code: ags
Overlay.CreateRoomGraphical(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic, 0 /*transparency*/, true /*clone*/);
(This is since AGS 3.6.0, previously it was always cloned, even non-dynamic sprites, which could cause lots of redundant sprite copying.)

This behavior is documented in the CreateGraphical's article:
https://adventuregamestudio.github.io/ags-manual/Overlay.html#overlaycreategraphical

This won't work with other game objects though: for them you'd have to save dynamic sprite in a global variable, like you seem to do with Overlays.

jumpjack

Quote from: Khris on Tue 03/01/2023 17:28:42If the first line is inside the function then flipped ceases to exist when the function finishes.
It's news to me that AGS keeps a reference to the original sprite and falls back to it but maybe that's how it works internally.

Anyway, the solution is to add
Code: ags
DynamicSprite* flipped;
above the function, then do
Code: ags
  flipped = DynamicSprite.CreateFromExistingSprite(tileIdNorm,  true);
inside.
The code snippet I pasted above is taken as-is from my script: all rows are one after the other, all inside the same function; moving the declaration out of the function as first line of the script does not help. The sprite IS drawn and it DOES remain there after function ends... but it's the wrong sprite!



jumpjack

Quote from: Crimson Wizard on Tue 03/01/2023 17:28:57When you assign DynamicSprite to an object, it normally is not copied, but referenced by the sprite ID (this is their Graphic property). Like Khris mentioned above, in AGS script dynamic sprites, like overlays, are deleted when the last pointer to them is removed, which happens when the function ends in your case. This is similar to Overlays. As soon as dynamic sprite is destroyed, the game object looses it and switches to sprite 0, which is historically used as a "placeholder".
Can a warning message be added in the editor console? This is an "autonomous behaviour", I would expect a "null pointer error" rather than automatically changing to default pointer, but if the "autonomous behaviour" is designed to prevent game crash, maybe a warning to developer would be enough.

Quote from: Crimson Wizard on Tue 03/01/2023 17:28:57
Code: ags
Overlay.CreateRoomGraphical(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic, 0 /*transparency*/, true /*clone*/);
Thanks, it works.


Crimson Wizard

#5
Quote from: jumpjack on Tue 03/01/2023 18:48:15The code snippet I pasted above is taken as-is from my script: all rows are one after the other, all inside the same function; moving the declaration out of the function as first line of the script does not help. The sprite IS drawn and it DOES remain there after function ends... but it's the wrong sprite!

This does not sound normal, I expected Khris's code to work.
If the wrong sprite is one with ID = 0, that's a placeholder, which might mean the dynamic sprite was deleted.
Can you post a code that you tried according to the Khris's advice (and which still did not work)?

Quote from: jumpjack on Tue 03/01/2023 18:52:07Can a warning message be added in the editor console? This is an "autonomous behaviour", I would expect a "null pointer error" rather than automatically changing to default pointer, but if the "autonomous behaviour" is designed to prevent game crash, maybe a warning to developer would be enough.

Adding a warning is a good idea. I don't think editor is currently programmed to receive runtime warnings like that (this may be another future feature); but the warnings about potential game mistakes are written into the "warnings.log" file found in Compiled/Windows folder when the game is run with F5 from the editor.
(and also in the general engine log too)

Khris

Very weird.

Are you positive what you're seeing is the overlay? What happens when you comment out all other drawing commands? None of this makes sense to me.
AGS shouldn't be able to draw the overlay after the function ends, but instead of no longer drawing the flipped one it draws the original one...?

I means as far as I can tell, if your original code had worked as intended you'd have created the overlay exactly on top of the sprite you're drawing on the background, which already raises one or two questions for me.

To clarify: what you describe is the sprite's non-flipped version (overlay) appearing right on top of the flipped version (part of background), correct?

Can you post a screenshot? Or better, upload a demo that replicates this?

jumpjack

Quote from: Crimson Wizard on Tue 03/01/2023 19:00:19
Quote from: jumpjack on Tue 03/01/2023 18:48:15The code snippet I pasted above is taken as-is from my script: all rows are one after the other, all inside the same function; moving the declaration out of the function as first line of the script does not help. The sprite IS drawn and it DOES remain there after function ends... but it's the wrong sprite!

This does not sound normal, I expected Khris's code to work.
If the wrong sprite is one with ID = 0, that's a placeholder, which might mean the dynamic sprite was deleted.
Can you post a code that you tried according to the Khris's advice (and which still did not work)?


It's exactly the code I posted in first message; to be sure, I did split first line in declaration part and assignment part, and moved declaration to first line of script, out from any function, so  I have:

Code: ags
DynamicSprite* flipped;
...
...
other things
...
...
function room_Load(){
...
...
      flipped = DynamicSprite.CreateFromExistingSprite(tileIdNorm,  true); 
      flipped.Flip(eFlipLeftToRight); 
      roomOverlay[overlayIndex] = Overlay.CreateRoomGraphical(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic);
      roomOverlay[overlayIndex].ZOrder = isoX + isoY;
      background.DrawImage(screenx - tilewidth/2, screeny - 2*tileheight -3,  flipped.Graphic);
...
...
}

What is really weird is that flipped.Graphic does contain the right value (65), and slot 65 does exist, because it works fine AFTER overlay.CreateRoomGraphical, in DrawImage.


jumpjack

Note:
the background.DrawImage statement is there just for debugging, to see if I was doing something wrong with overlay: it draws exactly what the overlay statement should draw (flipped sprite), but it has no real use, because I need the player to pass behind the drawing, hence I need an overlay.

My source is currently a mess :-) , anyway how do I upload a .zip to the forum?

Khris

You upload it elsewhere, then post the link.
OneDrive, Google Drive, Dropbox, Mediafire, etc.



Khris

#13
The extremely crucial part that is missing from this thread is the fact that the code runs in a loop.

Combine this with the fact that an Overlay does not by default create a copy of the sprite and the reason for the bug is obvious: reusing the same single sprite pointer for several overlays like this means there will only exist two dynamic sprites, in this case #65 and #66.

The first free slot is 65, so running CreateFromExistingSprite() for the first time creates a new dynamic sprite in slot 65.
In the next loop iteration the command runs again, the first free slot is now 66. At the same time though slot 65 is freed because the only pointer to it has just been overwritten. So when the line runs for the third time, it re-uses slot 65.

The resulting overlays will eventually contain arbitrary sprites, arbitrarily flipped or not, since all overlays with the exception of the last two are still using a sprite slot that contains a completely unrelated sprite.*

Debugging this was not hard, I simply added a line that displays flipped.Graphic and moved the code to room_AfterFadeIn.

What I don't get is why all these overlays do not change their sprite as soon as the slot is repopulated with a new sprite, so it looks like the graphical overlay does somehow buffer the sprite even without the clone parameter?*

*I'm still not sure what exactly is happening tbh. But it's pretty safe to say that this is caused by user error, not an engine bug.

Edit:
I created a test game and created three overlays in a loop, using a single DynamicSprite pointer. Without cloning, the result is as expected: only the third overlay uses the sprite correctly, the other two appear as blue cups.
Flipping the 1st and 3rd sprite first also gives the expected result: it works fine if the overlay clones the sprite, without cloning I again get two non-flipped blue cups (because being flipped is a property of the Dynamic Sprite, not the overlay).

Crimson Wizard

Quote from: Khris on Thu 05/01/2023 11:52:00What I don't get is why all these overlays do not change their sprite as soon as the slot is repopulated with a new sprite, so it looks like the graphical overlay does somehow buffer the sprite even without the clone parameter?*

*I'm still not sure what exactly is happening tbh. But it's pretty safe to say that this is caused by user error, not an engine bug.

Could you clarify this; do you know if it's possible to reproduce this with a simpler code?

Some sprite and texture caching exist in the engine, so it's hypothetically possible that it is not cleared on sprite deletion too. (That happened with room objects before)

Khris

I'm pretty sure I just missed something; I looked at the code again and it branches inside the loop, also creating overlays from static sprites. Just a few of them are flipped and they basically all end up being blue cups as far as I can tell.

jumpjack

Whatever is happening here (and I don't understand anything yet  ;) ), I think the main issue is that the engine should not go on with an arbitrary value for the sprite: a program should never act "on its own", inventing a pointer!

Possibly this mechanism is a precaution introduced to prevent game from crashing in production, but I think it's mandatory for the editor to warn the developer that there is a serious error (a missing pointer!) not managed by developer. The final user would only see a weird unexpected object on the screen, and the game would not crash, but a wrong item on the screen possibly  could even prevent the user from finishing the game: he would get stuck for unknown reason, and this would make him VERY sad, and he would throw away the game. This could even happen in first room, and the game would never be played.

Khris

What's happening here is you tried to use a single DynamicSprite to create multiple persistent overlays all showing different sprites.
I get VERY sad when somebody blames the tools.

Crimson Wizard

#18
Well, display of the placeholder sprite could be confusing, but I explained what it means in my very first reply. Both original and following script versions seem to  contain logical mistakes, where overlays kept using a DynamicSprite(s) which went out of its (their) life scope.

But warnings regarding missing sprites should be added of course, imo that's an oversight that there was none at least printed to warnings log.

Snarky

Quote from: jumpjack on Fri 06/01/2023 08:47:50I think the main issue is that the engine should not go on with an arbitrary value for the sprite: a program should never act "on its own", inventing a pointer!

It's not an "arbitrary value," it's 0, aka null. (And it's not strictly speaking a pointer, but a table index.) That sprite 0 is bluecup, and that this is the default AGS sprite when no sprite has been set, is explained in the basic "How to use AGS" tutorial:

QuoteAs if by magic, a blue cup appears! This is the default sprite in AGS, and will appear anywhere that you haven't selected a proper image yet.

It wouldn't make sense for the engine to crash whenever a DynamicSprite is deleted that is used as a graphic by some in-game entity, because it could very well be that this entity is not currently visible, or is just about to update to use some other sprite, etc. Users would have to manually keep track of and remove any reference to its sprite ID before deleting it, which would be extremely tedious. The chosen solution is textbook, simple and effective, and requires only minimal competence with AGS to understand.

In any case, this is not the root of the problem here. But sure, keep blaming AGS for your own mistakes.

SMF spam blocked by CleanTalk