Overlays not being removed?

ID:141195
 
May 28 2009, 4:38 pm
Code:
obj/points
    icon =  'lp_display.dmi'

    bg
        layer = FLOAT_LAYER-4

    ring
        layer = FLOAT_LAYER-3
    
    numbers
        layer = FLOAT_LAYER-2
    
    symbol
        layer = FLOAT_LAYER-2

mob/proc/LPRefresh(lptype)
    if(!src) return

    overlays -= typesof(/obj/points)

    var
        slot1
        slot2
        slot3
        slot4
        slot5
        U = 0
        X = 0
        Y = 0
        lptext = num2text(round(LP))

    switch(dir)
        if(NORTH)
            X = -32
        if(SOUTH)
            X = 32
        if(EAST)
            X = -16
            Y = -16
        if(WEST)
            X = 16
            Y = -16

    if(LP>99999)
        slot1 = "9"
        slot2 = "9"
        slot3 = "9"
        slot4 = "9"
        slot5 = "9"

    else
        if(LP<9999) U =- 2
        if(length(lptext)<5)
            var/max=5-length(lptext)
            for(var/i=0,i<max,i++) lptext="*[lptext]"
        slot1 = copytext(lptext,1,2)
        slot2 = copytext(lptext,2,3)
        slot3 = copytext(lptext,3,4)
        slot4 = copytext(lptext,4,5)
        slot5 = copytext(lptext,5,6)
    
    var/obj/points/symbol/nLP = new/obj/points/symbol
    nLP.icon_state = "lp"
    nLP.pixel_x = X
    nLP.pixel_y = Y
    overlays += nLP

    var/obj/points/bg/BG = new/obj/points/bg
    BG.icon_state = "lp_bg[rand(1,3)]"
    BG.pixel_x = X
    BG.pixel_y = Y
    overlays += BG
    
    var/obj/points/ring/RN = new/obj/points/ring
    RN.icon_state = "lp_[lptype]"
    RN.pixel_x = X
    RN.pixel_y = Y
    overlays += RN

    var/obj/points/numbers/N1 = new/obj/points/numbers
    N1.icon_state = "[slot1]"
    N1.pixel_x = X+6+U
    N1.pixel_y = Y-13
    overlays += N1

    var/obj/points/numbers/N2 = new/obj/points/numbers
    N2.icon_state = "[slot2]"
    N2.pixel_x = X+10+U
    N2.pixel_y = Y-13
    overlays += N2

    var/obj/points/numbers/N3 = new/obj/points/numbers
    N3.icon_state = "[slot3]"
    N3.pixel_x = X+14+U
    N3.pixel_y = Y-13
    overlays += N3

    var/obj/points/numbers/N4 = new/obj/points/numbers
    N4.icon_state = "[slot4]"
    N4.pixel_x = X+18+U
    N4.pixel_y = Y-13
    overlays += N4

    var/obj/points/numbers/N5 = new/obj/points/numbers
    N5.icon_state = "[slot5]"
    N5.pixel_x = X+22+U
    N5.pixel_y = Y-13
    overlays += N5

    spawn(20) if(src) overlays -= typesof(/obj/points)

    return


Problem description:

The overlays are not being removed.
I have tried doing "overlays -=" of the one of the objects instead (nLP / BG / N1 etc) and that worked... yet for some reason the typesof() fails.

I'm told you cant do typesof() with overlays, but ive done this in another proc and it worked fine. The only difference was that i was making the overlay directly (+= /obj/type/something/thing) then removing all of that type later in another proc (-= typesof(obj/type/something)).

Anyone know why this is happening? :/
#1 May 28 2009, 4:47 pm
everything in the overlays list is just a generic /image
#2 May 28 2009, 4:53 pm
Since they weren't added to the overlays list as an object type, when you try to remove them, BYOND doesn't identify them as being the same as what it's trying to remove, and so it skips them, leaving the overlays there. This is why when you add them as object types, they can be removed via typesof. Not the same thing here.

Fortunately, what you can do is simply this:

for(var/o in overlays)
    if(o:icon=='overlays.dmi')
        overlays-=o


Replace 'overlays.dmi' with the icon file you use for your overlays.
#3 May 28 2009, 4:57 pm
In response to Falacy (#1)
Well thats confusing..

As are the overlay examples ive found in the libraries, with people adding tags and removing the thing by 'icon.dmi'..

My old method of removal was to do;
for(var/X in overlays) if(X:icon == 'icon.dmi') overlays -= X


Which worked fine, but i know is not technically the right way to do it. I have also used typesof() elsewhere and it works too... which is kinda odd?

FlipMonsterOver(obj/cards/C)

        if(!C.FaceDown) return

        C.overlays -= typesof(/obj/overlays/cback)

        C.name = initial(C.name)
        C.FaceDown = 0
        CardEnteringPlay(C)

        for(var/mob/M in world)
            if(src in M.SeeDuel)
                M << sound('play_monster.ogg',0,0,1)
                M << {"<center>FLIP</center>"}

        return


As the cback overlay that was added could be cb1, cb2 or cb3. Anyway as i said this worked, but the method of adding it was different.

C.overlays += text2path("/obj/overlays/cback/cb[rand(1,4)]")


How would you suggest i remove it then?
#4 May 28 2009, 4:59 pm
In response to Devourer Of Souls (#2)
Thanks, well that explains why it worked before and not now.

lol @ your example though, i just posted as much below in a reply to falacy. Thanks anyway. :)

I was trying to do it the 'official' way as i was told my way (that way you posted too) was bad. Seems its the only option though (unless i add them directly)
#5 May 28 2009, 6:50 pm
In response to UnknownDuelist (#4)
Yes, you should definitely try to do it normally instead. The reason that doesn't work for you is that you're adding objects with a changed, custom appearance to overlays, and then trying to remove them by using the types of the objects, which have their compile-time appearance. So you're really trying to remove something different than what you've added - but to remove an overlay, you need to use an appearance identical to what you've used to add it in the first place. I'll illustrate: 
obj/thing
   icon = 'thing.dmi'
   icon_state = "thing"

mob/proc/AddOverlays()
   var/obj/thing/O = new
   O.icon_state = "something"
   src.overlays += O //essentially, you're adding the appearance of the 'something' state here
   sleep(30)
  /*now you're trying to remove the compile-time appearance
of the /obj/thing type - this will attempt to remove only
the "thing" state, so it won't do anything*/
   src.overlays -= /obj/thing
   //to successfully remove the overlay, you need to remove the same appearance you've added...
   src.overlays -= O
#6 May 28 2009, 6:54 pm
In response to Falacy (#1)
I don't see why'd you say that anyway, but it's quite false, either way. The overlay list's contents are in a special internal format, really not /image objects.
#7 May 28 2009, 9:42 pm
As others have said, overlays isn't made up of atoms, but /image datums. A trick I learned from OneFishDown is to make your own list, call it Overlays, manipulate that one the way you'd like, and then set overlays=Overlays.
#8 May 29 2009, 1:43 am
In response to Airjoe (#7)
Airjoe wrote:
As others have said, overlays isn't made up of atoms, but /image datums.

Not exactly. There are no datums in it, rather everything added is converted to an internal format, objects that are called "Appearances", and you can't do anything with them other than read some of their vars* (though it's still useful). They're not of the same type you get with new /image(), so saying they're images is pretty misleading. If you loop through the overlays with for(var/image/I in overlays) you're going to loop through nothing.
*: If you read the type of one, it does return /image, but this is of course more arbitrary than indicative. Also, they probably gave those objects this internal name before they implemented the /image objects we know.

A trick I learned from OneFishDown is to make your own list, call it Overlays, manipulate that one the way you'd like, and then set overlays=Overlays.

Hmm, I always saw it more as a way of running from the original problem and a probably unnecessary workaround, rather than some nice 'trick', but to each his own, I guess. =P
#9 May 29 2009, 5:44 am (Edited on May 29 2009, 6:23 am)
In response to Airjoe (#7)
Airjoe wrote:
As others have said, overlays isn't made up of atoms, but /image datums. A trick I learned from OneFishDown is to make your own list, call it Overlays, manipulate that one the way you'd like, and then set overlays=Overlays.

What Joe said is about the best option there is.

mob
 var/list/Overlays=new/list
 proc/AddOverlay(obj/o)
   Overlays+=o
   overlays=Overlays.Copy()
 proc/DelOverlay(obj/o)
   Overlays-=o
   //same
#10 May 29 2009, 6:01 am
In response to Mysame (#9)
Mysame wrote:
What Joe said is about the best option there is.

I really wouldn't assert that. As I've already hinted, I don't find it necessary, if you just know how to work with overlays and you do things right. Naturally, I could be wrong though!

Also, about your example code:
proc/AddOverlay(/obj/o)

That's incorrect argument declaration syntax and of course won't compile right; just pointing that out.

    for(var/i in Overlays) overlays+=i

You should be able to just do Overlays += overlays instead for the exact same (but more efficient) effect, just like with every list.

Also, a centralized RebuildOverlays() proc would be more... centralized, and also useful regardless of whether or not you use a dummy "Overlays list" implementation.
#11 May 29 2009, 6:08 am
In response to Kaioken (#5)
I guess then what i could do is create these objects totally (which will be a somewhat long and pointless list, but oh well) then i could avoid using new() so typesof() would work again (or just -= /obj/thing too). Thanks for clearing it up a bit more though.
#12 May 29 2009, 6:23 am
In response to Kaioken (#10)
Right, sorry, missed the error. And about handling lists, I'm still getting used to that :p Been a whole revelation for me, discovering lists can be used in that way.
#13 May 29 2009, 6:55 am
In response to UnknownDuelist (#11)
UnknownDuelist wrote:
I guess then what i could do is create these objects totally

Not sure what you mean here.

then i could avoid using new() so typesof() would work again (or just -= /obj/thing too). Thanks for clearing it up a bit more though.

Yes, if, when you add them to overlays, your objects are still with the same appearance as they were in compile-time, you can remove them from overlays with their type path. Illustration: 
mob/verb/1_sec_overlay()
   var/obj/some_type/O = new()
   src.overlays += O
   sleep(10)
   src.overlays -= /obj/some_type // (src.overlays -= O.type)

The above will normally work (it could fail, if /obj/some_type has an unexpected New() override that changes its appearance after its creation). Some perhaps-more-illustrative-illustrations: 
obj/some_type
   icon = 'Ball.dmi'
   icon_state = "red"
//______________
mob/verb/1_sec_overlay_Fail_Version()
   var/obj/some_type/O = new()
   O.icon_state = "blue"
   src.overlays += O

   src.overlays -= /obj/some_type //this will NOT work
//but this will work
mob/verb/1_sec_overlay()
   var/obj/some_type/O = new()
   O.icon_state = "blue"
   O.icon_state = "red"
   src.overlays += O

   src.overlays -= /obj/some_type

It's important to understand this generally works the same, for example vice-versa: 
obj/some_type
   icon = 'Ball.dmi'
   icon_state = "red"

mob/verb/1_sec_overlay()
   src.overlays += /obj/some_type

   var/obj/some_type/O = new()
   O.icon_state = "blue"
   src.overlays -= O //will NOT work
   O.icon_state = "red"
   src.overlays -= O //WILL work


With that out of the way... I'm not sure if what you've did is designed right or not and all that, but as for your problem of how to go about removing the overlays, what Devourer Of Souls suggested is not a bad idea. Just make sure you don't really have other uses for the same specific DMI file (lp_display.dmi), i.e. that you don't use it for any kind of other overlays. That will prevent possible issues.
Anyway, 
mob/proc/Remove_LP_Overlays()
   /*the /image typecast is for convenience _only_,
you could use virtually anything instead, like /atom, /mob... you could also use the : operator instead*/
   for(var/image/X as anything in src.overlays)
      if(X.icon == 'lp_display.dmi')
         src.overlays -= I

This should work perfectly for removing all the overlays that have that exact same icon file. You can also check other vars like the icon_state and pixel_X* if needed, but it doesn't like it is.
*Misc note: Last time I checked, reading the pixel_X vars there didn't work, but it's supposed to have been fixed at one point[link], or will be in the future.
#14 May 29 2009, 7:27 am
In response to Mysame (#12)
Mysame wrote:
And about handling lists, I'm still getting used to that :p Been a whole revelation for me, discovering lists can be used in that way.

Happy to revelate. =P Uh, or, teach you something new or whatever (provided I did).
I see you've edited it to a different method now; 
overlays=Overlays.Copy()

And I suppose it's worth pointing out that another way to do the same thing here is just overlays = Overlays; but this is only because overlays is a special var. Normally if you do this with 2 normal vars, it would just set the first var to the same list (object) as the other var, however the overlays var automatically converts whatever given to it into its own list of special values in an internal type, which is why the above is enough to make the 2 vars remain referring to 2 independent lists.
#15 May 29 2009, 10:35 am
In response to Kaioken (#14)
I'm aware, but it looks fancy.
So it can't go wrong.
Ever.