What's wrong with this code?
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:
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)
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
DynamicSprite* flipped;above the function, then do
flipped = DynamicSprite.CreateFromExistingSprite(tileIdNorm, true);inside.
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.
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.
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
DynamicSprite* flipped;above the function, then do
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!
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:57Overlay.CreateRoomGraphical(screenx - tilewidth/2, screeny - 2*tileheight -3, flipped.Graphic, 0 /*transparency*/, true /*clone*/);
Thanks, it works.
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)
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?
Quote from: Crimson Wizard on Tue 03/01/2023 19:00:19Quote 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:
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.
(double post)
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?
You upload it elsewhere, then post the link.
OneDrive, Google Drive, Dropbox, Mediafire, etc.
[deleted]
Opened issue, with screenshots and full code:
https://github.com/adventuregamestudio/ags/issues/1882
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).
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)
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.
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.
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.
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.
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 (https://adventuregamestudio.github.io/ags-manual/acintro4.html):
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.
Quote from: Crimson Wizard on Fri 06/01/2023 10:08:59But warnings regarding missing sprites should be added of course, imo that's an oversight that there was none at least printed to warnings log.
I'm not convinced. Consider something like:
DynamicSprite* dsFluid;
// ...
for(int i=0; i<simulationSteps; i++)
{
fluidSimulation.AdvanceSteps(1);
dsFluid = fluidSimulation.Render();
btnDisplay.NormalGraphic = dsFluid.Graphic;
Wait(1);
}In other words, a button displays a dynamic sprite, and this sprite is updated to do some animation (by switching the pointer to a new dynamic sprite rather than by redrawing onto its drawing surface). In this case, the old dynamic sprite no longer "exists" (refcount==0) as soon as
dsFluid is reassigned, and so
btnDisplay.NormalGraphic is invalid at that point. Of course, we're
just about to update it in the next line.
Should AGS throw a warning (or actually,
simulationSteps warnings) about that? Seems like it would clutter up the logs.
Quote from: Snarky on Fri 06/01/2023 14:39:47Should AGS throw a warning (or actually, simulationSteps warnings) about that? Seems like it would clutter up the logs.
Yes, that's a valid point. This would require a somewhat different approach then, but I'm not sure which at the moment.
For example, engine could only save the information, but process it only when updating/redrawing the game - for the objects that still don't reference anything,
Anyway, that's completely separate topic, and I would not want to clutter this forum thread. There was already a ticket opened regarding this: https://github.com/adventuregamestudio/ags/issues/1880
Quote from: Snarky on Fri 06/01/2023 14:18:06Quote 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.
[/quote]
If 0 and null are considered "the same", we have a serious problem here.
Anyway I leave the discussion to AGS experts, because I found the solution for my script.
Quote from: jumpjack on Fri 06/01/2023 16:43:08Quote from: Snarky on Fri 06/01/2023 14:18:06It's not an "arbitrary value," it's 0, aka null.
If 0 and null are considered "the same", we have a serious problem here.
Hmm. I have no idea why Snarky even mentioned "null" in this context, as the sprites in Overlays and other game objects are referenced not by a pointer, but by an integer (sprite ID).
No, "null" is not the same as "0" in AGS script; even though it was originally based on C programming language, but it ended up something like a mix of C and C#. "Null" may be only applied to pointers. Other variables cannot be assigned "null", and pointers cannot be assigned "0" and other numbers.
In AGS script non-pointers cannot be nullified (or "undefined"), like in javascript, for example, - they always have a value.
I don't like excessive log messages and checks that are not necessary that will only reduce performance. There's no need for it. This will only reduce log usefulness.
Also 0 was used in place of null in optional parameters - before some change in the compiler. Not sure if it was zero instead of null a long time ago somewhere.
0 is not a blue cup, it's the 0 sprite, which is ignored in some cases (GUI background and button graphics), and has a special meaning. It happens in the sierra template the sprite 0 is a blue cup.
Quote from: Crimson Wizard on Fri 06/01/2023 17:41:51Hmm. I have no idea why Snarky even mentioned "null" in this context, as the sprites in Overlays and other game objects are referenced not by a pointer, but by an integer (sprite ID).
Just that "null" literally means zero, and null pointers are typically implemented under the hood as a pointer with a value of zero. So while they apply to different types, there is a fundamental connection between
null and 0. Setting references to nonexistent sprite IDs to 0 is therefore loosely analogous to setting a pointer to null (of course with the important difference that 0 is in fact a valid sprite ID for an existing sprite, unlike null pointers, which have no existing referent).
Quote from: eri0o on Fri 06/01/2023 18:33:04Also 0 was used in place of null in optional parameters - before some change in the compiler.
As of the current official release I'm fairly sure it still is, though that's not why I made the comment.
[DELETED] because I should not continue discussing unrelated things here.
I added further info and a screenshot in the issue, but it is closed and discussion should continue here:
https://github.com/adventuregamestudio/ags/issues/1882#issuecomment-1374435160
I'm sorry, but can you clarify what you're asking for exactly?
The solution to this issue is to either
a) create an array of dynamic sprites so each overlay has its own
b) use the clone option so each overlay has its own
The github post seems to just restate the original problem, which in my book is thoroughly analyzed and solved already?
Quote from: Khris on Sat 07/01/2023 11:26:19The solution to this issue is to either
a) create an array of dynamic sprites so each overlay has its own
b) use the clone option so each overlay has its own
I might add, that you also may assign same dynamic sprite to multiple overlays, so long as you keep that dynamic sprite saved somewhere (in a global variable, or inside a global array) and
not overwritten.
Since you are generating maps using same "tiles library", you may also use a slightly different approach.
Make an array of dynamic sprites, containing only necessary
variants. You may precreate it once at the game start. Then, when you are creating overlays on a map, simply use dynamic sprites from that array.
Such approach should improve performance too.
Quote from: Khris on Sat 07/01/2023 11:26:19I'm sorry, but can you clarify what you're asking for exactly?
The solution to this issue is to either
a) create an array of dynamic sprites so each overlay has its own
b) use the clone option so each overlay has its own
The github post seems to just restate the original problem, which in my book is thoroughly analyzed and solved already?
my script is working fine, I don't need any further help, thanks.