Overlay enhancement: opinion on script wanted

[Crimson Wizard]

There have been some improvement to Overlays in 3.6.0, and generally improvement to perfomance of GUI and overlays since 3.5.1, I've been also looking for other opportunities to make Overlays more useful in the long run at a low effort cost (since 3.6.0 is in Beta, I would not want to do any serious code rewrite in this version). There are couple of ideas that technically should not be difficult to implement, but would require some good solution script-wise. Even if these won't get into 3.6.0, this still would be interesting to discuss.

These ideas are:
1) Overlays without sprite copy, but with a sprite reference;
2) Room Overlays.



1. Overlays with a sprite reference

Right now Overlays work with a full sprite copy. I guess this may be because they were first created as textual objects, and later expanded to also have sprite on them.
This makes it easier to create textual overlays, as you just have to pass a string into Overlay.CreateTextual. This also makes it somewhat easier to create graphical ones with Dynamic Sprites, as you don't have to keep that sprite anymore: as soon as overlay is created - it has a sprite's copy, and dynamic sprite may be disposed. Unlike that, if you're using dynamic sprites as object graphic or view frame, you must keep that sprite in a global variable at all times, otherwise the object will loose its graphic (in the past it also crashed the game, but not anymore: now it simply resets it to sprite 0, for safety).

Unfortunately, such approach backfires when you are reusing the sprite on several objects, in other words - either a) displaying same sprite often, and especially if b) using multiple overlays to display a larger numbers of identical images. Overlay itself is a simple struct, but each time when created it also copies the sprite. That makes its creation slower, and increases the overall memory use. In fact, it also increases the disk space needed for a save, as all overlays must write their bitmaps into the save file; which is quite silly if you used a regular sprite from the game resources to create them.
This is even more sad as overlays today are the only object in AGS that may be created at runtime in script. On one hand you may use it to generate graphic scenes on the fly, on another - it is not as optimized as it could be.

There's another consideration here. As you might know, AGS uses "sprite cache", that is - it caches often used sprites in memory to keep them ready and not load/allocate over and over again. Unfortunately, it does not do the same to textures when running Direct3D or OpenGL renderers. That is a general issue, and rather annoying one, as it makes these renderers actually somewhat slower* and more memory consuming than they could be (*slower in one thing, but thankfully they still balance this with being faster in others). I think that one of the future improvements (and quite overdue) is to also cache the textures prepared for particular sprites. If that's done, then whenever you assign a sprite repeatedly, not just the sprite won't be reloaded, but also the texture won't be recreated but taken from the cache instead and shared between multiple objects.

But even if that's done, with overlays things would remain the same, so long as it's function is based on a sprite copy.

So, what is that script problem in this situation? Technically there's little issue: just don't make a image copy and add a sprite reference instead.
But if this is done, that would change the known overlay's behavior, where user may paste a dynamic sprite over it, and discard the sprite object. Instead they would have to make global variables for these sprites and keep a record on them, deleting these when no longer necessary, etc.
Perhaps this is also a problem with the dynamic sprite management in AGS, as their references are not contained in some storage where they will be "safe" from occasional loss, but must be referenced by a script variable.

EDIT: Another difference between having a sprite's copy and a reference is that you may continue editing same sprite after assigning it to a game object. If an object has sprite's copy - then it won't receive further changes. If it has a reference - then it will be automatically updated as soon as you finish editing the sprite (call to DrawingSurface.Release()). /EDIT

What could be a solution here script-wise, if we would like to allow Overlays with a sprite reference instead of a copy? Should there be a new function, that explicitly sais that it's not copying the sprite? Or, on contrary, a function that sais that it makes a copy, while CreateGraphical would make a reference? Are there any alternate ideas maybe?

In regards to dynamic sprites, that may be a separate question (or is it?), but one idea is to treat their assignment to an object as a proper counted reference, thus making them not automatically disposed while their number is assigned to at least one game object.



2. Room Overlays

IMHO this is a interesting idea. Overlays are unlimited (since 3.6.0), easy to make in script, but they are always on top of the room. They received ZOrder property in 3.6.0, so now may be freely sorted among GUI.
In my opinion, it's technically not hard to allow have them inside the room, where they sorted among the room elements (objects, characters and walk-behinds). This opens very good opportunities for setting up temporary visual effects inside the room, as well as generate room scenes in script. Especially since other objects are fixed, and cannot be created or deleted at runtime right now (that would require significant change to how AGS is designed, and is a separate big topic).

But the question that I have is again, how to organize this in script to make convenient? Having a boolean argument in CreateGraphical / CreateTextual is simple, but may be annoying. It may be an enum, which means "game layer" or something (like, eRoomLayer, eGUILayer). Alternatively, these might be separate pair of functions, e.g. Overlay.CreateRoomTextual and Overlay.CreateRoomGraphical.

What do you think?

[Crimson Wizard]

Well, is someone wants to try out the experimental build, here's room overlays support:
https://cirrus-ci.com/task/6659508301725696

Room overlays are created using new pair of functions:

Code: ags

  /// Creates an overlay that displays a sprite inside the room.
  import static Overlay* CreateRoomGraphical(int x, int y, int slot, bool transparent);
  /// Creates an overlay that displays some text inside the room.
  import static Overlay* CreateRoomTextual(int x, int y, int width, FontType, int colour, const string text, ...);

They are same overlays, but drawn and sorted in the room space using their ZOrder property. Unlike characters and objects they don't automatically change their order when moving along Y axis, so if you want them to do that, you have to write something like this in script:

Code: ags

function room_RepExec()
{
	if (roomover)
	{
		roomover.ZOrder = roomover.Y + roomover.Height;
	}
}

[eri0o]

I don't have much to add about 1 at the moment, but I reopened the original room overlay issue, which I had closed when we were at the initial implementations of the SDL2 port, when things were a bit uncertain.

I don't remember about Texture cache, if this was discussed here or separately, and I also couldn't find an issue specific for it, but I think 1 as is gets a bit faster if there's some way to do this.

[Crimson Wizard]

Having overlays in rooms was simple enough to make, and it opens many possibilities, so perhaps will be added to 3.6.0, if testing is successful.

I think I won't plan p1 for 3.6.0, because there are more things to consider there (like the dynamic sprite handling that i mentioned). And since it's already in Beta, so perhaps I'm already pushing it a bit too far with new features.

[abstauber]

About the "by reference or "by value" issue:

How about submitting a pointer to the function and overlays would be created by reference, whereas supplying a sprite slot would result in creating a copy.

[Crimson Wizard]

About the "by reference or "by value" issue:

How about submitting a pointer to the function and overlays would be created by reference, whereas supplying a sprite slot would result in creating a copy.

Which pointer? If you mean sprite's, there's no Sprite type in AGS to pass a pointer for, only DynamicSprite, but that would limit this to dynamic sprites only. Unless we create Sprite type to address all the sprites.
In general i think such approach would be confusing to people, and inconsistent with the rest of the script commands, where assigning a sprite slot does not create a copy.

[abstauber]

Yeah, maybe my active scripting time are too long ago. I had something in mind, similar to how PHP would do it
https://www.php.net/manual/en/language.references.pass.php

But I agree, this would differ a lot from the rest of the script commands.

[eri0o]

Was just playing around with Room Overlays, figuring out what could be done with unlimited things in the room!

[Crimson Wizard]

I'm thinking that the most straightforward idea for at least a temporary solution for p1 (graphic sharing) would be to just add a parameter for CreateGraphical that tells "make copy". Make it false by default, as in - make a reference. For example:

Code: ags

Overlay.CreateGraphical(int x, int y, int slot, bool transparent, bool clone = false);

The reason why I think it should "not clone" by default is, to prevent unnecessary resource load without user realizing that.
This would work seamlessly for normal sprites. For dynamic sprites, people might stumble onto this once if they delete the sprite too early, or change it to suddenly see the change in overlay as well. This is solved by learning about the parameter.

The old games run in the new engine would have to "make copy" all the time, as they did not have such param.

Additionally, it would be possible to optimize this by (implicitly) not making copy if we know it's not a dynamic sprite, because normal sprites cannot be changed or deleted.
At first I thought this might be the best approach without extra script parameters. But then I thought that in theory a game may have a collection of generated dynamic sprites applied to overlays, like some kind of a tilesheet or something, in which case making copies would also be not wanted.



UPDATE: Hmm, this appared to be rather promising. At least it works. Even if I'd only make an implicit rule to not make copies of a regular sprites without changing script api - that already should improve perfomance when there's alot of overlays with similar sprites on screen.

[eri0o]

Question on cloning. Say in the future somehow is decided that the Graphical property (the sprite id) must be also a writable property, if the Overlay was created using a clone flag, does it mean it should copy on change and not really be a reference in that case? I am wondering if an Overlay should have an additional property for the Overlay type (say textual, graphical clone, graphical... ?)

[Crimson Wizard]

Question on cloning. Say in the future somehow is decided that the Graphical property (the sprite id) must be also a writable property, if the Overlay was created using a clone flag, does it mean it should copy on change and not really be a reference in that case? I am wondering if an Overlay should have an additional property for the Overlay type (say textual, graphical clone, graphical... ?)

I see cloning behavior as a historical bit that we are forced to keep for the time being, but may remove later (in ags4). In my opinion that should not be a part of an object's logic (otherwise we should add same to every object), but rather a part of a sprite management. So, if Overlay.Graphic is added at some point, imo that should be non-cloning reference always.

[Crimson Wizard]

I opened a PR if someone wants to test this out the CI server should make the build soon:
https://github.com/adventuregamestudio/ags/pull/1646

[Crimson Wizard]

There's one important question that I forgot about, regarding the room overlays: what should happen to a room overlay when the room changes.

Variants probably are:
* it is removed; meaning it exists so long as player is in current room.
* it is kept but is not displayed until returning to the same room; that is owned by room.
* it is kept and transferred to another room as-is. This is dangerous, as there's a real chance of loosing control over the overlay, if its pointer is only in the previous room script: then the new room has no means to control it.

NOTE: right now all overlays are removed forcefully when changing rooms, that's a standard AGS behavior.

[eri0o]

uhm... About overlays... How far off is adding tint to them? 

[Crimson Wizard]

uhm... About overlays... How far off is adding tint to them? 

Alan Drake is demanding that i dont add anything to 3.6.0 anymore, so all the additions go into ags4 now.
Right now tinting may be added through dynamic sprite's Tint function.

Technically, tint is applied to a texture, so it's a matter of having a parameter in overlay. However, in regards to ags4, I'd rather suggest moving towards base class, where it will contain all the transform and color effects. Then one won't have to add separate parameters to each type of object.

[eri0o]

Ah, ok, just thought to ask

I will use dynamic sprites for now. It was for lighting in this tiled fps test here: https://ericoporto.github.io/agsjs/galleys/

[Crimson Wizard]

I found certain behavior in 3.6.0 which may be seen as a design oversight, but I wanted to hear other peoples opinion on this too, before I do any changes.

Prior to 3.6.0 graphic overlays always made a copy of the original sprite. This was inefficient (but since overlay numbers were limited, it was not a very big deal), but also meant that if you create Overlay from the DynamicSprite, and later change DynamicSprite, then the overlay will not be updated.

Since 3.6.0 this is controlled by a parameter to CreateGraphical, but default is to not copy, but reference a sprite instead. This means that changing dynamic sprite will also affect overlays which reference it.

Something I missed is the resizing of DynamicSprite. It appears that if you do, the overlay will use new image but keep old size.

Fixing this may be trivial, but there's a logical conflict with the manual Overlay sizing, using Width & Height properties.

My thinking is that there are two options:

1. Require user to handle this. That is - don't resize overlay automatically, and let user decide whether to set overlay's size after resizing a dynamic sprite.
This is the current behavior, and may be left as-is.

2. Have 2 "modes", where overlay either follows sprite's size or keeps user-set size:
* Overlay remembers whether it had its size explicitly set by user script; upon creation it is in "auto" state and sizes to the sprite's size.
* If its sprite is resized, and it was never resized by user, then it will automatically change to the new sprite's size.
* If it was explicitly set size by user, then it keeps that size.
* Additionally, let user pass Width or Height as 0, which reverts Overlay to auto-resizing.

[eri0o]

I mostly prefer 1, but that's because of how I manage overlays currently in my scripts where I maintain an in script mirror of their properties and update only later to avoid script <-> engine calls which have some overhead and impact performance.