ID:1813930
 
Resolved
A new type, /mutable_appearance, can be used to modify multiple appearance values without creating a lot of temporary appearances, thus reducing churn and improving performance.
Applies to:DM Language
Status: Resolved (511.1348)

This issue has been resolved.
I know the likelihood of this making it into the language is slim, but this is a powerful little suggestion that I'm pretty sure would be hugely helpful for quite a few things that would otherwise be computationally expensive to perform at the moment.

It's my understanding that appearances more or less are tracked internally as either a struct or an object, and have a unique reference ID. Every time you change an atom's appearance, it checks the global appearance dictionary and looks to see if that unique combination of appearance values exists. If it doesn't exist, it creates a new appearance instance and communicates that appearance to connected clients.

Appearances seem to be refcounted just like any other object. They are immutable by default in DM, however. Attempting to locate() an appearance by it's reference gives you read-only access to the appearance.

I'm proposing that appearances finally be exposed to the developer for both direct (no-hack) reading and writing.

1) expose /appearance as a type to DM.

2) allow /appearance's variables to be modified. This would update the appearance object on the server and generate a network message.

This would modify the communication protocol to add a new message, which would communicate changes to appearance objects. This would basically be analogous to the existing appearance communication that's done to the client on a new appearance being sent in the first place, only instead of appending to the client's appearance dictionary, it would replace the old element.

3) finally, expose atom.appearance to DM, which gives you a direct reference to the atom's appearance object. This would simply be moving an existing field from the C side of things into the DM side of things, I'm assuming.


The benefits of this change to the server's appearance management would allow developers to implement global ambient lighting changes merely by changing a single appearance, rather than looping through hundreds of objects per connected player. This would also allow easy implementation of things like global season changes merely by looping over a much smaller number of appearances rather than potentially hundreds of thousands of objects.

Now, there are some potential complications: If you set an appearance to an identical appearance that's already in use, it could result in either duplicate appearances, or accidentally merging two appearances into one another. Those complications could be easily explained in the documentation, and could be easily designed around by the developer if they understood those complications. They might even be taken advantage of as "not bugs" (read: features) for interesting effects.

My suggestion for fixing those two bugs is to bring appearances in line with datums and give them a "tag" variable that counts as a part of its unique ID for indexing the appearances. That way, you can tag appearances you want to remain visually the same, but still have a single variable that remains different to avoid merging appearances you don't mean to.

And finally, here's a basic code example:

var
list/seasonal_appearances = list()
season = "summer"

proc
change_season(Season)
var/endl
for(var/appearance/a in seasonal_appearances)
endl = length(a.icon_state) - length(global.season)
if(endl)
a.icon_state = copytext(a.icon_state,1,endl) + Season
else
a.icon_state = Season
sleep(-1)
global.season = Season

season_ticker()
set waitfor = 0
sleep(6000)
change_season("fall")
sleep(6000)
change_season("winter")
sleep(6000)
change_season("spring")
sleep(6000)
change_season("summer")
spawn() season_ticker()

world
New()
season_ticker()

turf
seasonal
var
appearance_tag = null
New()
icon_state += season
appearance.tag = appearance_tag
var/val = seasonal_appearances[appearance]
if(val)
seasonal_appearances[appearance] = val+1
else
seasonal_appearances[appearance] = 1
Del()
var/val = seasonal_appearances[appearance]
if(val>1)
seasonal_appearances[appearance] = val-1
else
seasonal_appearances.Remove(appearance)
appearance.tag = null
Ideally, also, mutating resources would be a better approach, but mutable appearances would be useful as well.

This is of course, assuming that the client actually indexes resources according to their reference number (which it seems that they do), and the renderer seeks the file via an abstracted reference on render every frame during render.

For the webclient, you would swap the texture handles in OpenGL. Stage3D at some point should be storing and referencing a GL memory location for textures. Switch that around and you've got yourself a resource swap without updating every object that references the resource.

Mutable icons could be approached via a function:

rsc_replace(OldResource,NewResource)
It's an interesting idea. I don't love the notation, because it is inconsistent with the rest of DM and causes some weird thinking where a change to one variable changes a bunch of others. I do see what you're getting and and I agree that we would like to have some improvements to the client-side functionality.

It seems to me that a better system would be for the ability to declare a special type of atom variable whose members, outside of the location, are static. So you could assign a mask to every object and update only a single instance to handle lighting changes. This isn't really new to DM-- this is how areas work, but I assume areas are too limiting for everything you want to do here. Or we could kill two birds with one stone and expand /image to be a lot more flexible, with one option being this static mode.

This is all hypothetical, of course, but maybe there's a good solution here that will accommodate power users while also not being so low-level in notation.
As it stands, there does exist a static keyword for variables. Unfortunately, the compiler seems to believe that static is an analogue for global. It makes sense, given how DM types are managed and maintained internally (more like metatables than true classes). It's just rather peculiar semantics compared to other OO languages.
Thinking about this a bit more, if the primary need is just mass updating icon_state, as in your example, it seems this could be done cleanly be encapsulating it within /icon (where it logically belongs anyway, but isn't for purposes of convenience) AND allowing atom.icon to properly sync with instance assignments. Eg:
var/icon/I = new('icon.dmi')
I.icon_state = "summer"
var/mob/M1 = new()
var/mob/M2 = new()
...
M1.icon = M2.icon = I
...
// update both M1 and M2
I.icon_state = "winter"
I.Turn(180) // intuitively, this would work already but it requires a resync, which defeats the purpose


All but the I.icon_state part exist in the notation already, but it doesn't work as expected, presumably because atom.icon assignment is a copy and not a pointer (which seems counter-intuitive to the notation). I don't think there is any functional reason it has to be this way, though. Perhaps this is something we can fix.

Ideally though, I'd really like to expand the /image routine to expose more details about appearances to give a finer degree of control within the language.

Of course, if you want to efficiently change more than just the icon-- eg, the atom name or the click behavior, etc, then the above isn't sufficient. But it seems to me that most people just want better lighting control.
M1.icon = M2.icon = I


You are getting to comfortable with Dart's syntactic sugar, there Tom. Unfortunately that doesn't compile in DM. ;P

That aside, I mean, I guess that would work more or less for the purposes I initially envisioned.
I'm not really sure this is of a lot of practical benefit, though. It seems to me that the main issue with client-side lighting is that generally there are a lot of dynamic icons-- icons that only the client really cares about-- and these have to be generated for each player on the server. Even sending over icon_state changes for a 1000 icons shouldn't really be a huge overhead. Of course this is just talking out of my ass without profiling it.

If we really want to be competitive as far as client-side stuff we probably need give the client better access to atoms so it can override atom appearances. Realistically, this would have to be through javascript since we don't have a DM-side parser in Dart. I think a lot of this information is actually readily available (Lummox JR would know more), so maybe it's possible if we can come up with a decent notation for it. But this is something I wouldn't even consider until the other parts of the system are better optimized. We want to first get to the point where it's as good as DS, and then we can try to surpass it.
I don't see mutable appearances working out for a lot of reasons--and it's something I've thought about before. I think supporting it would be kind of hellish.

However, I do think it'd be a worthwhile idea to expose an appearance var for all atoms and images, and make that settable. Reason being, it'd be easy to copy any appearance:

var/obj/clone = new
clone.appearance = usr.appearance

That would copy all appearance-related values at once, which is something that I think comes up a lot more often. Also, this would make it fairly easy to modify an overlay.

var/image/temp = new
temp.appearance = usr.overlays[1]
temp.dir = EAST
usr.overlays[1] = temp
That would copy all appearance-related values at once, which is something that I think comes up a lot more often. Also, this would make it fairly easy to modify an overlay.

I'd basically kill for the ability to do that.
I'm moving forward with adding an appearance var to 508. It will copy over everything but density and verbs, which really aren't as relevant to the appearance as to the object.
Lummox JR resolved issue with message:
A new type, /mutable_appearance, can be used to modify multiple appearance values without creating a lot of temporary appearances, thus reducing churn and improving performance.
Why couldn't this have been called something like:
/image/mutable, considering it's literally a special case of /image ?
In response to Super Saiyan X
Super Saiyan X wrote:
Why couldn't this have been called something like:
/image/mutable, considering it's literally a special case of /image ?

Because that belies it's reason for existing, it exists to counter appearance churn, to drop appearance from the name hides this.
I would have preferred /appearance/mutable, but that would have led some people to assume /appearance was a type they could work with in a similar way.
In response to Lummox JR
I always thought it was interesting how you could set the "loc" var for /atom/movable but not for /atom. I think if you were to use /appearance/mutable, you would have to also include an /appearance type whose variables are read-only, like loc is for /atom; but are not read-only for /appearance/mutable, like loc is for /atom/movable.

But, would getting atom.appearance result in an /appearance object, or an /appearance/mutable object?
In response to Lummox JR
Lummox JR wrote:
I would have preferred /appearance/mutable, but that would have led some people to assume /appearance was a type they could work with in a similar way.

is there any reason we don't have an /appearance type? we can already access variables from appearances using : which feels dirty and not at all safe.
is there any reason we don't have an /appearance type?

Because they cannot be instantiated or destroyed by the developer.

They are handled pretty much internally and to boot, this conversation with Tom/Lummox is more or less the starting point for many of our recent appearance-related conversations on the site:

http://www.byond.com/forum/?post=1847214

This conversation resulted in that feature request being fulfilled, IIRC.

I dunno, appearances are just sort of black magic right now. mutable appearances will hopefully see some use by developers going forward, but you never know. It may be too wonky for many to understand.
I suppose the concept of a read-only typepath is a bit odd and unfitting for DM.
Why does the appearance var only change some values? It would be nice if we could just set the appearance to another one with 0 churn.

since per the stddef.dm;
mutable_appearance
parent_type = /image
_dm_interface = _DM_datum|_DM_image|_DM_Special

I think what I'm gonna do...

appearance
parent_type = /image
mutable
parent_type = /mutable_appearance

because luls
Page: 1 2