ID:1429662
 
(See the best response by Nadrew.)
Code:
        Nobuild()
set category = "Icon Commands"
switch(alert("Would you like to turn nobuild on or off?","Nobuild Options","ON","OFF","Cancel"))
if("ON")
for(var/obj/O in world)
if(O:owner=="[src.key]")
O:nobuild="ON"
return
if("OFF")
for(var/obj/O in world)
if(O:owner==src.key)
O.nobuild="OFF"
return


Problem description:

Why isn't this doing squat? I've double checked and something that was coded similar works fine.

I checked to see if it was alert's problem, and still nothing. Am I doing something wrong and I am just having brain farts, or is this correct and something is wrong with DM?



For one, you definitely shouldn't be using the : operator here, the . operator should work perfectly fine unless you've defined the owner variable poorly.

Next, you should probably be double-checking that O.owner is being set as expected when the object is created, putting in some debugging output would probably help you a whole lot.

Lastly, looping over every object in the world like this probably isn't a great idea, you should probably be tracking the objects created by specific users in a list of some kind. There's also a hidden benefit to this, saving a list of object references to a savefile magically saves those objects and will load them when the list is loaded again. Of course you'd have to track location your own way since x, y, and z aren't automatically saved for you.

Something like

mob/var/list/mystuff = list()

obj
var
mob/tmp/owner // Best not to save this.
owner_key // This'll be saved, instead of a reference to the player which creates issues


mob/verb/BuildSomething()
var/obj/new_something = new(usr.loc)
new_something.owner = usr
new_something.owner_key = usr.key
if(!usr.mystuff) usr.mystuff = list() // Just in case
usr.mystuff.Add(new_something)


The reason I have both a reference to the owner itself and the owner's key as variables is because you don't want the owner getting saved with the objects, this can cause many, many headaches, but you want to be able to re-establish that reference after loading the object so you save the key and go from there after the object is loaded.

Now when you want to access all of the objects a person has created, just loop over their 'mystuff' list, a whole lot more resource friendly than a global lookup.

I'll let you figure things out from here, if you need more to go on just lemme know; this is just a solid starting point.
That wasn't the problem at all to be honest. Sorry, I actually fixed it like an hour ago and I just remembered to go on here and tell everyone that. Thank you for showing me some things.

All I had to do was this.


        Nobuild()
set category = "Icon Commands"
switch(alert("Would you like to turn nobuild on or off?","Nobuild Options","ON","OFF","Cancel"))
if("ON")
for(var/obj/O in world)
if(O.nobuild=="OFF")
if(O.owner!="[src.key]")continue
if(O.owner=="[src.key]")
O.nobuild="ON"


Thanks anyways. ;/



I need to explain this verb.

The No Build verb allows the player to put a lock on their objects so if others invaded their build or whatever, they won't be able to spam build on that object.

A proc communicates with no build by a variable that switch to a value of 1 which means "You are not allow to build."
Best response
I wasn't indicating what the problem with your code was as far as your original query, I was indicating that the code itself is flawed and horribly inefficient in comparison to the alternative I gave you.

Say you want to output a message to the owner of something, you'd have to loop over all the players to find the one with the matching key, then output the message to that player; my system would let you just output it to owner without any looping (aside from a single loop when the object loads to reestablish reference ownership).

There's a similar issue with your Nobuild() command, any time any player uses it the game will loop over every single object in the game, whether that player owns it or not, that will cause major overhead if say, more than one person uses the command, or someone decides to use the command repeatedly in a short amount of time. The example I gave you opened the door to not only cut potentially thousands of objects out of each call to the verb, but prevent the need to loop elsewhere needlessly.

You have to remember that when you use for() you're not just instantly accessing everything in that loop, the more it has to iterate over the longer it'll take to finish and the more resources it'll use as it does so. At some point if your game garners a high player count and many players are building things you're gonna end up with a major bottleneck in loops like this. A good standard to follow is to avoid looping 'in world' any time you can avoid doing so, which you most definitely can in this instance; rather easily at that.

Using my example your NoBuild() verb could be rewritten as such:

Nobuild()
set category = "Icon Commands"
var/newtoggle = alert("Would you like to turn nobuild on or off?","Nobuild Options","ON","OFF","Cancel")
if(newtoggle == "Cancel") return
for(var/obj/O in mystuff)
O.nobuild = newtoggle


The for() here will not only be MUCH faster, but it'll be MUCH more resource-friendly when many people are using the command at the same time. There's also no need to do an owner check (wouldn't hurt though if you want to) because 'mystuff' will always only contain things the player has built themselves. You'll also notice than since your 'nobuild' variable accepts the same values as alert() returns you can just use that without any conditional checking of the value. Ideally, though you'd want to make nobuild a boolean true/false value for more efficient if() checks on it, but that's not a major thing like looping over the world each time you use a specific verb ;).

I also noticed something that you should probably break the habit of early on, notice how when you're checking the value of O.owner you have two if() statements that are basically opposite of each other? This is where the 'else' statement comes into play, but you don't even need that here at all, you only need the one check, either one would do it. There's no need to see if owner matches src.key if the conditional of it not matching already pulls true, the same true for the other if().

if(owner == src.key)
// Do stuff


No need for anything else, since nothing will happen if that if() fails.
You know what...I greatly agree with you.

I've been influenced with my way and haven't come to think of it why most games have so much lag.

It would be very friendly to servers and it will communicate much more efficient...OK, I am doing it. ;)