ID:2116980
 
Resolved
Finalized mutable appearances were discarded too early, causing crashes in some cases.
BYOND Version:511.1348 + 511.1349
Operating System:Windows 7 Home Premium 64-bit
Web Browser:Chrome 51.0.2704.103
Applies to:Dream Daemon
Status: Resolved (511.1350)

This issue has been resolved.
Descriptive Problem Summary:
While testing some features of 511 I produced the code below (snippet section), I'm aware it's not great/safe (: operator abuse) but it's a simplified version of the code that initially caused the crash, it was simply an attempt at copying an appearance into a mutable appearance (Not sure if there's actually a proper way to do this)

In the snippet section are 4 verbs, the first verb will crash Dream Seeker 100% of the time, the following 3 are variants of the above with certain lines removed or altered (as commented), All 3 of the variant versions work perfectly fine causing no crash.

As shown in the category of the verbs and this thread's title I'm inclined to believe it's tied to BOTH mutable appearances and pixel_w, as pixel_w was the only var I could use to crash it, and even then it has to be obtained from another appearance using :

tl;dr pasting the 4 snippet below into Dream Seeker and testing the verbs should explain it all. (You will have to restart Dream Seeker a minimum of 4 times as 3 of the times remove your verbs preventing you testing the others and the other is an actual crash)

Code Snippet (if applicable) to Reproduce Problem:
/mob/verb/mutapp_crash() //Crashes DreamSeeker
set category = "pixel_w + Mutable Appearances = Crash"
set name = "Crash"

var/mutable_appearance/MA = new()
MA.pixel_w = appearance:pixel_w //variable obtained from src's appearance
for(var/a in MA.vars)
world << "[a] = [MA.vars[a]]"
appearance = MA


/mob/verb/no_mutapp_crash() //Does not crash DreamSeeker
set category = "pixel_w + Mutable Appearances = Crash"
set name = "No Crash - No var dump"

var/mutable_appearance/MA = new()
MA.pixel_w = appearance:pixel_w //variable obtained from src's appearance
appearance = MA


/mob/verb/no_mutapp_crash2() //Does not crash DreamSeeker
set category = "pixel_w + Mutable Appearances = Crash"
set name = "No Crash - Integer pixel_w"

var/mutable_appearance/MA = new()
MA.pixel_w = 32 //A number
for(var/a in MA.vars)
world << "[a] = [MA.vars[a]]"
appearance = MA


/mob/verb/no_mutapp_crash3() //Does not crash DreamSeeker
set category = "pixel_w + Mutable Appearances = Crash"
set name = "No Crash - No Appearance Assignment"

var/mutable_appearance/MA = new()
MA.pixel_w = appearance:pixel_w //variable obtained from src's appearance
for(var/a in MA.vars)
world << "[a] = [MA.vars[a]]"


Expected Results:
Set the pixel_w value of a mutable appearance to the one found in src's appearance using the : operator, output all the variables the mutable appearance has and the variables' values, then assign src's appearance to the mutable appearance.

Actual Results:
Dream Seeker crash.

Does the problem occur:
Every time? Or how often? Every time, without fail
In other games? Any game that uses this snippet of code, I assume
In other user accounts? Assume yes
On other computers? Assume yes

When does the problem NOT occur?
As shown in the examples, it's a very specific bug

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked?
Prior to 511, obviously, as neither mutable appearances or pixel_w existed

The verb panel is removed because assigning a /mutable_appearance to an atom.appearance also changes the verbs list.
In response to Super Saiyan X
Ah yes, I forgot appearances held those variables, primarily because when I was testing I just quickly skimmed the ref page which distracted my mind with the nice bright popping hyperlinked variables and assumed those were the only ones (Where the others are mentioned in a paragraph below this)

It's not relevant to the actual issue report, but I can eliminate those from the report now, so thanks.
Also, I just tested this, and I think it's got nothing to do with pixel_w. It doesn't 'crash' for me either - just hangs. Unresponsive to any commands.

It seems to be a combination of reading mutable_appearance variables (to the viewer), and then assigning that object.

mob
verb
//crash
blank_appearence_output()
var/mutable_appearance/MA = new
for(var/a in MA.vars) world << "[a] = [MA.vars[a]]"
appearance = MA

//no crash
blank_appearence_no_outout()
var/mutable_appearance/MA = new
//for(var/a in MA.vars) world << "[a] = [MA.vars[a]]"
appearance = MA

//no crash
blank_appearence_no_assign()
var/mutable_appearance/MA = new
for(var/a in MA.vars) world << "[a] = [MA.vars[a]]"
//appearance = MA

In response to Super Saiyan X
Care to explain /mob/verb/no_mutapp_crash2() then?
it avoids the crash solely by using a number for pixel_w, as opposed to setting it to src's appearance's pixel_w

And for me it's definitely a crash, it hangs for a couple seconds as you say, and then stays in "Not Responding" with the little windows popup.
in no_mutapp_crash2(), MA isn't a blank appearance, at least one of it's variables has been altered.

When you do MA.pixel_w = appearance:pixel_w, appearance.pixel_w is still 0.

Like,
        Not_a_blank_appearence()
pixel_w = 32
var/mutable_appearance/MA = new
MA.pixel_w = appearance:pixel_w
for(var/a in MA.vars) world << "[a] = [MA.vars[a]]"
appearance = MA

This doesn't crash or hang for me, and in this case - MA is no longer a blank appearance; it had it pixel_w copied from src.appearance:pixel_w (which is == to src.pixel_w)

Thus, this issue seems to only exist for appearances that are unaltered.
Lummox JR resolved issue with message:
Finalized mutable appearances were discarded too early, causing crashes in some cases.