Picking up Items (Fishing Rod Specifically)

ID:142311
 
Jun 25 2008, 12:12 pm
Code:

obj
    items
        verb
            Get()
                set src in oview(1,usr)
                set category=null
                usr<<"<b><font size = 0.5><font color = green>You pick up [src.name].</b>"
                src.loc = usr
                if(src.name == "Fishing Rod")
                    usr.hasfishingrod = 1
                if (src.name == "Fishing Rod" && usr.hasfishingrod == 1)
                    usr<<"You already have one rod, you don't need a"
                    usr<<"<b><font size = 0.5><font color = green>You drop [src.name].</b>"
                    src.loc = usr.loc
                    
            Drop()
                set src in usr.contents
                set category=null
                usr<<"<b><font size = 0.5><font color = green>You drop [src.name].</b>"
                src.loc = usr.loc
                if(src.name == "Fishing Rod")
                    usr.hasfishingrod = 0
        staticitems
            fishingrod
            name = "Fishing Rod"
            icon = 'Fishing Rod.dmi'


Problem description:Yeah... so every time I try to pick up the fishing rod, it brings me to this (Even if I don't have one in my inventory:
You pick up Fishing Rod.
You already have one rod, you don't need a
You drop Fishing Rod.

Jun 25 2008, 12:42 pm
I'd completely get rid of the hasfishingrod variable. You can use the following code to check just as easily:
//If this is true, they have a fishing rod in their contents.
//If not, then they don't.
if(locate(/obj/item/fishingrod) in contents)

Now, probably the easiest way to write a get verb would be as such:
obj/item

    verb/Get()
        //The verb is active within one tile of the object,
        //and not on the object.
        set src in oview(1)

        //Set the location in the usr's contents.
        src.loc = usr

    verb/Drop()
        //This makes the verb active when the object is in
        //the user's contents. At this point, Get() should
        //no longer work.
        set src in usr.contents

        //This makes the object's location the location of
        //the usr.
        src.loc = usr.loc
Jun 25 2008, 12:43 pm
Problems:

--Identifying objects by their name is bad. Use their type instead. Look up the 'type' var and the 'istype()' proc in the DM Reference.
Also, you aren't checking if the player already has a fishing rod or anything, you are just checking if the picked-up-obj's name is that of a fishing rod, so obviously the if() always executed the code when a fishing rod is picked up regardless of the player not having one already.

--Don't use /mob boolean variables such as 'hasfishingrod'. This is simply badly designed, not to mention you're going to have a variable for each item type in your game. Instead, when wanting to check if the player has a fishing rod in him ( == in his contents == in his "inventory"), call the locate() proc (look it up) and check if it can find a reference to the object.
** Besides, since your proc is already built in an
Object Oriented Programming method, you should simply
override the proc for the types you wish to have
special handling with, instead of checking for them
in the general/main version of the proc. Example: 
obj/item
  verb/Get() //initial proc (verb) definition
    set src in oview(1)
    if(src.Move(usr)) //if the pick-up (movement) succeeded
       usr << "You pick up \the [src]." //notify the usr
  fishing_rod
    Get() //proc override
       if(locate(src.type) in usr) //if an object of src's type (ie /obj/item/fishing_rod) is found in usr already
           usr << "You already have \a [src]"
       else //if one wasn't found
          ..() //call the parent proc - means, execute the above Get()
//this means the player is only allowed to pick up a fishing\
rod if he doesn't have one already, rather than the bad \ design of picking up and immediately dropping. \
if you have multiple similar restrictions for other items \
as well, a different, more generally flexible design should\
be in order


-As seen in the above example, Move() is better used for moving objects than setting the loc directly. If the items are non-dense, it will [noticeably-wise] behave pretty much as setting loc, both-ways. Also, you should make use of DM's handy text macros as seen above (look up 'macros (text)' for more info) to ensure proper grammar.

--Don't put spaces between HTML attributes and their values (ie, don't put color = red but color=red.

--Tip: This is also something important to remember in the future: usr is view()'s (and similar procs) default Center argument. This means you can write only oview(1) and it will be the same as oview(1,usr).

--Tip: Also, the default src setting for /objs is src in usr, so you don't have to specify it.

Good luck!
Jun 25 2008, 1:59 pm
In response to Kaioken
Code:
obj
    items
        verb
            Get() //initial proc (verb) definition
                set src in oview(1)
                if(src.Move(usr)) //if the pick-up (movement) succeeded
                    usr << "You pick up \the [src]." //notify the usr
            Drop()
                set src in usr.contents
                set category=null
                usr<<"<b><font size = 0.5><font color = green>You drop [src.name].</b>"
                src.loc = usr.loc
        fishingrod
            Get() //proc override
                if(locate(src.type) in usr) //if an object of src's type (ie /obj/item/fishing_rod) is found in usr already
                    usr << "You already have \a [src]"
                else //if one wasn't found
                    ..()
        staticitems
            fishingrod
            name = "Fishing Rod"
            icon = 'Fishing Rod.dmi'


New Problem:

Okay, I took your code for a test drive, and would modify it to fit my needs afterward (A learning experience, as I see it anyway. Hope to learn from your example), and it doesn't shift to the override Get() verb when using a fishingrod. It always uses the original for some reason, which I cannot see.

Probably me being an idiot again though.
Jun 25 2008, 2:31 pm
In response to Alaris123
Alaris123 wrote:
Code:
>       fishingrod
>           Get() //proc override
>               if(locate(src.type) in usr) //if an object of src's type (ie /obj/item/fishing_rod) is found in usr already
>                   usr << "You already have \a [src]"
>               else //if one wasn't found
>                   ..()
>       staticitems
>           fishingrod
>           name = "Fishing Rod"
>           icon = 'Fishing Rod.dmi'
>

You're still using the old fishing rod. Place an /obj/item/fishingrod on the map and try with it instead of /obj/item/staticitems/fishingrod.
Jun 25 2008, 3:16 pm
In response to YMIHere
.......
I can't believe I was so stupid...

THANK YOU! I can't believe I missed that!
Jun 25 2008, 4:38 pm
In response to Alaris123
Alaris123 wrote:
Okay, I took your code for a test drive, and would modify it to fit my needs afterward (A learning experience, as I see it anyway. Hope to learn from your example

You better. =P I didn't make it for the purpose of you forking it over to your DM file and skipping paying attention to my post, which also included other things. The best way to learn from it would probably be to rewrite it (to your needs as well) after reading my post, then when you finish comparing it for any things you might have missed. Copying isn't too productive.
Jun 25 2008, 5:01 pm
In response to Kaioken
Right right, I can understand that.

And I did look over the rest of your post (It's not like I just copied your code and ignored the rest... that would be rather pointless, as I'd learn nothing).

Thanks again.
Jun 25 2008, 5:07 pm
In response to Alaris123
No problem. Was just making sure to make sure (;P), as you probably figure what the common reaction to finding both text and code in a post is, even more so when there is more text.