Visibility Extension

Page: 1 2
ID:2197362
 
Jan 7 2017, 3:42 am (Edited on Jan 7 2017, 9:29 pm)
Visibility Extension

Currently in DM your options for handling visibility of objects are mostly limited to being on or off. Some situations call for more fine grain control where you only want some players to be able to see an object.

This gets a little more tricky as well when players enter and leave the game so you have to remember what's visible to who.

My idea was to standardize how we deal with these problems by extending our notion of visibility with a few helpful methods which handle it all for you.

Basic Example
var/obj/Apple/A = new/Apple()

A.HideFrom(usr) // Everyone can see the apple except the usr

sleep(5) // Delay

A.ShowTo(usr) // Now the usr can see the apple again


Healthbar Example
var/obj/HealthBar/HB = new/obj/HealthBar()

HB.HideFromAllOnline(usr)  // Anyone currently online won't be able to see it

HB.HideFromAllOffline(usr) // Even offline players won't see it when they come back online

HB.ShowTo(usr) // The current user can see it now

usr.overlays += HB


Source Code

You might only need a few of these methods but all are included for completeness.

/*
    Author: http://www.byond.com/members/Zecronious
    Date: 7 Jan 2017

    Description: Control what players can see what objects.
*/

atom
    var
        image/cover
        hideFromNewClients = FALSE

    New()
        ..()
        cover = image(null,src)
        cover.override = TRUE
        
        // Ensure that if this object is saved and recreated that it will
        // be added back to the global list so no information is lost.
        if(hideFromNewClients)
            hiddenFromNewClients += src

    proc
        // Specify one person to show this to
        ShowTo(target)
            if(istype(target,/mob))
                var/mob/M = target
                M.client.images -= src.cover

            else if(istype(target,/client))
                var/client/C = target
                C.images -= src.cover

        // Specify one person to hide this from
        HideFrom(target)
            target << src.cover

        // All players currently online will be able to see this
        ShowToAllOnline()
            for(var/client/C in clients)
                ShowTo(C)

        // All players currently online won't be able to see this
        HideFromAllOnline()
            for(var/client/C in clients)
                HideFrom(C)

        // Players who are offline will be able to see this when they come back online
        ShowToAllOffline()
            hiddenFromNewClients -= src

        // Players who are offline won't be able to see this when they come back online
        HideFromAllOffline()
            hiddenFromNewClients += src


var/list
    hiddenFromNewClients = list()
    clients = list()

client
    New()
        ..()
        clients += src
        for(var/atom/A in hiddenFromNewClients) A.HideFrom(src.mob)

Final Note

I've done the best I can do justify all my additions performance wise. This code leans more towards using a tiny bit more memory to save time on the CPU later on. Memory is cheap and people have lots of it but CPUs are expensive so I hold back as much as possible on that.

Also thanks to Gooobai for inspiring me to write this after I saw his cool idea for blind spots (http://www.byond.com/forum/?post=2186744)
Jan 7 2017, 4:56 am
You shouldn't be initializing hiddenFromNewClients the way you are, it'll create a bit of extra overhead and cause an init() proc to be added to the profiler which can get confusing when tracking down resource usage.

You should be initializing it as needed and nulling it back out when it's empty. You can also save a bit more overhead by using += (or |=) and -= instead of Add() and Remove().

You could save a bit of overhead on the loops by not doing a containerless loop (which defaults to world + special lists) by storing online clients in a list and removing them when they exit the game, and looping over that list.

Other than that, using image.override for this is a great technique.
Jan 7 2017, 4:59 am
In response to Nadrew
I agree with most of that but I'm a little confused about what you meant with hiddenFromNewClients. It's a global list so should only be initialized once?
Jan 7 2017, 5:03 am
Keeping initialized lists loaded up when they're empty is simply a waste of memory. It's perfectly acceptable to keep a single list in memory that way though, but you should still be initializing it in world/New() or the first time code that uses it encounters it set to null, even if you don't actually reset it to null when it's empty.

That way it's only loaded up if it's needed, and it prevents the init() proc overhead and profiler confusion.
Jan 7 2017, 5:07 am
In response to Nadrew
All right, world.New() I can understand. What I don't get is initializing it when it's used. That would require an if statement every time you want to add something to that list which would be extra CPU usage? Surely it's better to have an empty list in memory in case it's needed rather than constantly check if it's been initialized?
Jan 7 2017, 5:16 am
The overhead of checking if a variable is null or not and initializing the list is minimal and that check should be in your code anyways as a sanity check. You shouldn't ever actually assume a list is initialized without an if() check on it unless you're a fan of strange runtime errors when things don't go according to plan.

It's more of a thing on lists that are contained by more things than being a single variable, but it's a good practice to get into regardless since you should be doing the check for initialization anyways.
Jan 7 2017, 5:33 am (Edited on Jan 7 2017, 5:42 am)
In response to Nadrew
I can understand that more for instance variables where yes you would have hundreds or maybe thousands of lists so it pays to always check what state they're in before you start using them because you don't know what life that object containing the list has had or what else has meddled with the list.

In this case though where it's a single list with a single purpose, honestly it doesn't bother me. In a more formal language I'd be able to seal it off so nothing else can use it but I just don't have the option here.

If at any point you're thinking about adding something for the sake of it adding it, chances are it's a style choice and my go-to style is a minimal style. I like easy to read code, simple and without redundancy and little to no comments because the code should just read well.
Jan 7 2017, 6:07 am
Code should also handle exceptions properly, code that can account for an exception should always do so.
Jan 7 2017, 3:06 pm
I'd actually rather it threw an exception instead of silently failed. Easier to debug that way.
Jan 7 2017, 3:50 pm
That's why you use try and catch, you want to catch the exceptions and handle them gracefully instead of letting a runtime error crash the stack and potentially hang the rest of the game up.
Jan 7 2017, 3:56 pm (Edited on Jan 7 2017, 4:15 pm)
As an optimization,
for(var/client/c in clients)
  HideFrom(c)

can be written as
clients << cover

by inlining HideFrom(c) and then replacing "for(var/client/c in clients) c << " with "clients << ".

Also, it looks like a debug message was left in client/New():
        for(var/client/c in clients)
            world << c

Also, I would rewrite
            if(istype(target,/mob))
                var/mob/M = target
                M.client.images -= src.cover

            else if(istype(target,/client))
                var/client/C = target
                C.images -= src.cover

to this: (comments to explain why, but you can leave them out)
// We're interested in getting a client to show an image to.
var client/client
if(istype(target, /client))
    client = target
else
    if(ismob(target))
        var mob/mob = target
        client = mob.client
    if(!client)
        throw EXCEPTION("Bad client.")

// Now that we have a client, we can show the image.
client.images += cover

(To be fair, you're going to get a "null.images" or similar runtime error anyway if the client isn't valid)

Similarly, HideFrom(target) could also throw an exception if target isn't a client or a mob with a client. I don't think the output operator does that already in this case, but maybe it should.
Jan 7 2017, 4:37 pm
In response to Nadrew
'Code should also handle exceptions properly, code that can account for an exception should always do so.'

That's a little bit absolutist though don't you think? I can think of plenty of situations that could use a try-catch block but don't need one. Proven by the fact that BYOND has survived a long time without exception handling until recently.

I would rather have 4 lines of easy to read code than 10 lines of verbose and unnecessary code simply because I'm a following a rule without being thoughtful about whether or not I really need it.

Exceptions are used for handling when you're unsure what someone is going to do with your code or what someone is going to input to the code.

None of those lists I've used will ever be null. I can confidently say that because I know exactly where they're used and under no circumstances have they ever been nulled. If someone chooses to null one of them they're going to get a runtime error which will gracefully be handled in the background and it'll be their own fault.
Jan 7 2017, 4:50 pm
Good thing you're not trying to provide libraries for people to use with that mindset.
Jan 7 2017, 4:53 pm
In response to Nadrew
Give me an example of what someone might do to break this and if it seems like a fair situation I'll remedy it with an exception.

Otherwise I think you're evangelizing exceptions where they aren't necessary. I've read code from people with that mindset and their libraries are spaghetti that only they can understand.
Jan 7 2017, 4:55 pm
In response to Nadrew
Littering code with exceptions is a great way to ensure future employment though...
Jan 7 2017, 5:35 pm
Just because this exact example doesn't have many fail cases, you should never assume the users of the library won't do something stupid, and your library shouldn't implode on itself if it comes across something unexpected, it should handle it gracefully and tell the developer why it didn't work as expected, not an obscure runtime error that may or may not give any detail as to what or where the problem is, and would crash the stack. Basic error handling is a cornerstone of programming, especially programming for others to use.
Jan 7 2017, 5:37 pm
And mind you, library standards here do expect you to gracefully handle error cases, libraries should never be able to crash with a runtime error. Things that came before the addition of try/catch were expected to do sanity checking where appropriate, and things after are expected to properly handle exceptions. They won't be published otherwise.
Jan 7 2017, 5:58 pm (Edited on Jan 7 2017, 6:06 pm)
In response to Nadrew
Nadrew wrote:
Just because this exact example doesn't have many fail cases, you should never assume the users of the library won't do something stupid.

I can do a great many things with my code but healing stupidity is up to God.

Look at my code. If you paste that code into your game I cannot see a single possible reason why my code would ever give a runtime error. However!... If a person decided to edit my code and change things then it is their responsibility to make sure their changes work. If they delete stuff that is in my code then I can no longer guarantee anything.

I task you with finding a library on BYOND that is impossible to break once you edit it. I then task you with finding a case where this snippit would ever break.

I don't think you can.
Jan 7 2017, 6:07 pm (Edited on Jan 7 2017, 6:58 pm)
In response to Kaiochao
Kaiochao wrote:
As an optimization,
for(var/client/c in clients)
  HideFrom(c)
can be written as
clients << cover
by inlining HideFrom(c) and then replacing "for(var/client/c in clients) c << " with "clients << ".

Also, it looks like a debug message was left in client/New():
        for(var/client/c in clients)
            world << c
Also, I would rewrite
            if(istype(target,/mob))
                var/mob/M = target
                M.client.images -= src.cover

            else if(istype(target,/client))
                var/client/C = target
                C.images -= src.cover
to this: (comments to explain why, but you can leave them out)
// We're interested in getting a client to show an image to.
var client/client
if(istype(target, /client))
    client = target
else
    if(ismob(target))
        var mob/mob = target
        client = mob.client
    if(!client)
        throw EXCEPTION("Bad client.")

// Now that we have a client, we can show the image.
client.images += cover
(To be fair, you're going to get a "null.images" or similar runtime error anyway if the client isn't valid)

Similarly, HideFrom(target) could also throw an exception if target isn't a client or a mob with a client. I don't think the output operator does that already in this case, but maybe it should.

since you have k-chow blocked
Jan 7 2017, 8:29 pm
I task you with finding a library on BYOND that is impossible to break once you edit it. I then task you with finding a case where this snippit would ever break.

Agreed with this. You shouldn't worry about what developers are going to do with your code. Only end users.
Page: 1 2