Object Check in Developer Help

ID:264576
 
Dec 7 2009, 6:51 pm
Code:
        Guild_Check()
            for(var/obj/Guild_Objects/Guild_Approved/G in world)
                src<<"Test1"
                if(G)
                    src<<"Test2"
                    if(G.name == src.guildname)
                        src<<"Test3"
                        return
                    else src.Give_Guild_Verbs("Disband")
                else src.Give_Guild_Verbs("Disband")


Problem description:
My guild system is saved off of obj's. I wanted to create a proc that will check if their guild's obj exists upon log in, so that if a person disbands their guild while a member is offline, it will notify the person when they log in and strip them of their guild crap. To seek out the bug, I set up three test strings, and you never make it past src<<"Test1"(as in Test1 doesn't appear). Can somebody tell me where I went wrong with for(var/obj/Guild_Objects/Guild_Approved/G in world)?
#1 Dec 7 2009, 8:10 pm
At first glance it looks like the Guild_Approved objects aren't being created. How are you handling their creation and persistence? If I recall correctly (and I might not), unless you are referencing the object somewhere and it's being used it will be processed by the garbage collection system.

Aside from that, you should handle most of the functionality within the object itself, and have a global handler datum keeping track of guild objects. I'll ignore the last part for now, but keep it in mind.

Having the object do the work makes your life a lot easier:
Guild_Check()
  for(var/obj/Guild_Objects/Guild_Approved/G in world)
    if(G) G.HandleMob(src)


Within G.HandleMob(mob/M) you'd take care of handing out verbs and notifications, rather than putting it all in the Guild_Check() proc.


A handler datum will make your life even easier.
GuildHandler
  var
    list/activeGuilds = list()

  proc
    AddGuild(obj/Guild_Objects/Guild_Approved/G) //Call from newly created Guild_Approved objects
      if(G && istype(G)) //make sure it's the type needed
        activeGuilds += G.name
        activeGuilds[G.name] = G

    GetGuild(guildname as text) //Return a Guild_Approved object based on the guild name. Will be null if guild doesn't exist in the activeGuilds list
      return activeGuilds[guildname]

    DelGuild(guildname as text) //Remove a guild that is no longer active. Useful when a guild has been disbanded and there are no more members.
      var/obj/Guild_Objects/Guild_Approved/G = activeGuilds[guildname]
      activeGuilds -= guildname //Remove the guild name from the list, if it exists
      if(G) del G //Delete the object, if it exists


With a handler object like above you wouldn't need to loop through the world and you wouldn't have an issue of persistence. A call to GuildHandler.GetGuild(src.guildname) would get you the exact guild object you need.



Note that the above are examples of concepts and may not function as presented. You should apply the concepts to your own code and not copy character for character.
#2 Dec 7 2009, 8:29 pm
In response to CriticalBotch (#1)
The whole process of creating the guild works just fine. I've tested it many of times.

obj
    Guild_Objects
        Guild_Approved
            icon = 'ObjsMisc.dmi'
            icon_state = "Guild"
            New()
                ..()
            var
                guildbase = 0
                guildpoints = 0
                background = "#000000"
                textcolor = "#FFFFFF"
                title = "N/A"
                maintext = "None"
                leader = null
                coleaders = list()
                generals = list()
                members = list()
                recruits = list()
        Guild_Unapproved
            icon = 'ObjsMisc.dmi'
            icon_state = "Guild Application"
            name = "Guild Charter"
            var
                guild_name = "N/A"
                guild_tag = null
                guild_leader = null
                Submit_Charter()
                    if(src in usr.contents))
                        if(!guild_leader)
                            usr << "<font color=red>Please fill out the Guild Name and Guild Tag you wish to have before submitting"
                            return
                        var/zane = 0
                        switch(alert("Are you sure you wish to submit your charter?\n- Note: You will not lose your Guild Charter if the guild is rejected.","Submit Charter","Yes","No"))
                            if("Yes")
                                for(var/mob/player/M in world)
                                    if(M.key == "Cyberlord34" || M.key == "Dark Prince X")
                                        zane ++
                                        sure:
                                        var/sure = input(M,"Do you wish to accept this guild?\n- Type 'Yes'  to Accept. Type No to Reject. Type 'Edit' to edit the names\n- Guild Name: [src.guild_name]\n- Guild Tag: [src.guild_tag]\n- Leader: [src.guild_leader]","Guild Application")as text|null
                                        if(!sure) goto sure
                                        if(sure == "Edit")
                                            var/guildname = input("What would you like to change the guild name to?","Guild Name")as text
                                            var/guildtag = input("What would you like to change the guild tag to?","Guild Tag")as text
                                            src.guild_name = guildname
                                            src.guild_tag = guildtag
                                            goto sure
                                        if(sure == "No")
                                            usr<<"<font color=red>Your guild has been rejected by [M]"
                                            return
                                        if(sure == "Yes")
                                            usr.guildname = src.guild_name
                                            usr.guildtag = "<font color=#66CD00>\[[src.guild_tag]]</font color>"
                                            usr.guild = 1
                                            usr.guildrank = "Leader"
                                            usr.guildlevel = 5
                                            var/obj/Guild_Objects/Guild_Approved/G = new()
                                            G.name = usr.guildname
                                            G.leader = "[usr] ([usr.key])"
                                            G.maintext = "Welcome to '[usr.guildname]', hope you enjoy your stay!"
                                            G.title = "[usr.guildname]'s Guild Page"
                                            G.loc=locate(185,210,10)
                                            usr.Give_Guild_Verbs("Leader")
                                            world<<output("<font color=#66CD00>Guild Alert: </font color>The '[usr.guildname]' lead by [usr] has been formed","Chat")
                                            del(src)
                                        else
                                            goto sure
                                if(!zane)
                                    usr<<"<font color=red>Your guild cannot be formed because Dark Prince X is offline"
                                    return
                            if("No")
                                usr<<"<font color=red>You have clicked 'No'. Process canceled"
                                return
#3 Dec 7 2009, 8:42 pm
In response to Dark Prince X (#2)
It may indeed work fine. That doesn't mean it works well. It also doesn't answer my basic question - what happens to approved guild objects after creation?

Are they just created and then float around?

I'm curious about your most recent code as well. I'm hoping it's just an indentation error from copy+paste, but you have your Submit_Charter() procedure defined as a variable? You also don't ensure that your alert is using src as it's target - that could lead to errors if Submit_Charter was ever called by another process. That goes for just about all uses of usr in that code bit as displayed.

And why are you using goto? Granted that goto may occasionally be the best option is some limited cases, it looks as though this is not one of them. Would a procedure call not be better?


Code quality aside, more information is needed about the process before a solution an be found. Looping through all the Approved_Guild objects in the world isn't working, which likely means there are no instantiated Approved_Guild objects in the game at the time. Until you can either prove this to be true or false you will not be able to continue tracking the issue to it's source.
#4 Dec 7 2009, 8:47 pm
In response to CriticalBotch (#3)
They are taken to a specific location. I have the saved in World/Del(), and loaded upon world/New(). Everything has been running smoothly, it's just this proc that won't work for some reason. I use for(var/obj/Guild_Objects/Guild_Approved/G in world) all the time, and it always works.
#5 Dec 7 2009, 8:50 pm
In response to CriticalBotch (#3)
Crap, I should have mentioned something. The object is being deleted on purpose before this proc is called. It's supposed to tell the person that the guild they were in was disbanded, so the object won't be there. What's supposed to be happening is if it can't find that specific object, it tells them their guild has been disbanded and takes away their stuff.
#6 Dec 7 2009, 8:56 pm
In response to Dark Prince X (#4)
Dark Prince X wrote:
They are taken to a specific location. I have the saved in World/Del(), and loaded upon world/New(). Everything has been running smoothly, it's just this proc that won't work for some reason. I use for(var/obj/Guild_Objects/Guild_Approved/G in world) all the time, and it always works.

Well, according to your own data, the issue is at
if(G)


Nothing is executing past that, so obviously G is null.
The way you acquire G is through
for(var/obj/Guild_Objects/Approved_Guilds/G in world)


Ergo, the break down is either in the loop (not likely) or the fact that G does not exist. Unfortunately I do not understand your process enough to speculate on where the break down is occurring. However, I can safely say that unless the loop itself is broken (which it should not be) then the problem is not in the proc that you originally requested help with. It is somewhere else in your code.

I would point out that just because something has always worked does not mean it is a good idea. I think this is a prime example of poorly executed design impacting future development. I would highly recommend redesigning your process to make use of a more object oriented approach. This would likely reduce the number of issues such as this as well as make it easier to track such problems down.
#7 Dec 7 2009, 8:59 pm
In response to CriticalBotch (#6)
Look at my more recent reply to your post.
#8 Dec 7 2009, 9:23 pm
In response to Dark Prince X (#5)
Dark Prince X wrote:
Crap, I should have mentioned something. The object is being deleted on purpose before this proc is called. It's supposed to tell the person that the guild they were in was disbanded, so the object won't be there. What's supposed to be happening is if it can't find that specific object, it tells them their guild has been disbanded and takes away their stuff.

Then let's look at this again:
Guild_Check()
    var/obj/Guild_Objects/Guild_Approved/CheckGuild
    for(var/obj/Guild_Objects/Guild_Approved/G in world)
        src<<"Test1"
        if(G && G.name == src.guildname)    //consolidated if statements
            CheckGuild = G                  //assigning the found object to the holding object
            break
            
    if(!CheckGuild) //guild doesn't exist (never assigned an object from the loop), handle clean up
    else            //handle what to do when the guild is found


As it loops through potential guild objects, what's happening? It checks the name against the character's guild name. If they match, output a debug message. If not, add a disband verb. If there's no guild object, add a disband verb.

No where in here are you telling the player that the guild no longer exists nor are you doing any clean up of guild related information and abilities.

There's a bigger issue, however. Consider the following:

There are three guilds in the game: A, B, and C

Let's go through the process.

The loop begins. First up is A.
All members of A are given a debug message.
All members of B and C are given disband verbs.
Loop. Now we're on B.
All members of B are given a debug message.
All members of A and C are given disband verbs.
Loop. Now we're on C.
All members of C are given a debug message.
All members of A and B are given disband verbs.

Now everyone had a debug message and had disband verbs added. Let's assume that you intend to handle the clean up either if there is guild or if the guild name doesn't match.

Well, now instead of getting disband verbs, everyone is now told their guild was disbanded and have all their guild related processes removed.

Again, having the ability to pull just the data you need would be a better option here - looping puts a higher strain on server resources than a simple procedure call and list look up.

However, in the interest of "fixing" the code version you seem determined to use, consider the following.

You need a way of considering only one object and handling output and clean up based on that object. The easiest way (but not the best, by a long shot) is to have a holding variable. Instead of doing all your checks on every object you loop through, you instead simply check if it's the object you're looking for. If it is, assign the object to the holding object and break the loop.

Guild_Check()
    var/obj/Guild_Objects/Guild_Approved/CheckGuild
    for(var/obj/Guild_Objects/Guild_Approved/G in world)
        src<<"Test1"
        if(G)
            src<<"Test2"
            if(G.name == src.guildname)
                CheckGuild = G
                break
            
    if(!CheckGuild) //guild doesn't exist, handle clean up
    else            //handle what to do when the guild is found


That should do what you're trying to do. Try an output message after if(!CheckGuild) - it should fire if G doesn't exist or if there are no guilds that match the player's guild name.
#9 Dec 7 2009, 9:30 pm
In response to CriticalBotch (#8)
Thank you very much. This worked perfectly.