Efficiency Help

ID:1525869
 
Mar 22 2014, 10:05 am
(See the best response by FKI.)
Code:
mob
    DblClick(mob/M in view())
        if(src == usr)
            return
        if(istype(usr.equip,/obj/Rifle))
            if(usr.ChkUse())
                for(var/obj/Rifle/R in usr)
                    if(!R.ammo)
                        view() << "\red\bold [usr]'s rifle clicks!"
                        return
                if(prob(rand(50,75)))
                    src.Dmg(rand(10,30))
                    if(!(locate(/obj/blood/) in src.loc)) new/obj/blood(loc)
                    view() << "\red\bold [usr] shoots [src] with a rifle!"
                    if(prob(25))
                        src.Bleed(rand(1,2),10)
                    goto ammo
                view() << "\red\bold [usr] misses [src]!"
                :ammo
                for(var/obj/Rifle/R in usr)
                    R.ammo--


Problem description:
It works fine, but currently I am only allowing one rifle in the inventory at a time. Is there a better way to rewrite this so that I can only decrease the ammo of the rifle the player is using? Like if there are multiple rifles in his inventory?
Mar 22 2014, 10:35 am
 
if(istype(usr.equip,/obj/Rifle))


Shows you already have an accessible way to identify which rifle is firing.

Just add:

var/obj/Rifle/R = usr.equip

Before the istype line and remove the for()'s.
Mar 22 2014, 12:39 pm
Best response
I suggest making use of polymorphism and creating a gun type.

gun
    parent_type = /obj
    var/mob/user    // hold a reference to the person holding the weapon in case you want to do animations or something in certain cases.
    var/rounds = 0   // amount of active rounds
    var/clips = 0    // amount of remaining ammo clips
    var/rounds_in_a_clip = 0

    Click()
        equip(usr)

    proc/equip(var/mob/holder)
        user = holder

        holder.equip = src
        holder.reload_overlays() // we'd be adding weapon overlays and refreshing appearance here.
        holder << "You equipped the [name]."


With this, you can handle things like shooting and reloading easily. It's a much more clean approach as opposed to throwing everything in DblClick. For example, here is what I wrote based on your post.

    proc/shoot(var/mob/target)
        if(rounds)
            . = 1 // ammo check complete.
            rounds = max(0, --rounds)
            if(!rounds)
                if(clips > 0)
                    reload()
                else
                    user << "You ran out of ammo!"
        else
            view(user) << "[user]'s gun clicks!"
            // returns null by default. The user will not successfully shoot.

    proc/reload()
        rounds = rounds_in_a_clip
        clips -= 1

    rifle
        rounds = 20
        clips = 1
        rounds_in_a_clip = 20

        shoot(var/mob/target)
            . = ..() // attempt to shoot.

            if( . )
                if(prob(rand(50, 75)))
                    var/damage = rand(10, 30)

                    target.Dmg(damage)

                    if(prob(25))
                        target.Bleed(rand(1, 2), 10)
                else
                    user << "You missed!"


Also, the DblClick trigger:

mob/var/tmp/gun/equip // I'd go with tmp for this, as I wouldn't save it. You may do your saving different, so something to consider.

mob/DblClick()
    if(usr == src) return 0

    if(usr.equip)
        usr.equip.shoot(src)

    . = ..()


Feel free to use what you want, but more importantly, make sure you realize what is going on in the code. A lot of things become a lot easier and overall cleaner when using this approach.

Edit: Also, try not to use goto. As your code grows, use of the aforementioned can get really clumsy. I'd just avoid it if I were you.
Mar 23 2014, 10:15 am
Thanks, I rewrote the code earlier without the goto, because it eas glitchy anyways. I will keep this in mind in the future! (I also thought about using a user defined datatype for this, and it sounds like a good idea)
Mar 23 2014, 10:26 am
Also, how would I write the code so that even if a mob is in view, it can't be hit, like through bulletproof glass.
Mar 23 2014, 10:41 am
In response to Vivalas
You'd have to get the turfs in between the atom shooting and the atom being shot, checking each turf in the process for blocks, whether that be bulletproof glass or a box.

This is taken from a project of mine using tile movement (I'm not sure if it will work if you're using pixel movement).

    get_line(atom/A, atom/B)
        if(!A || !B)
            return list()
        // Can't handle Z-level changes
        if(A.z != B.z)
            return list()

        var
            x0 = A.x
            x1 = B.x
            y0 = A.y
            y1 = B.y

        var/vertical = abs(y1-y0) > abs(x1-x0)
        // Mirror vertical lines, because it only works with "horizontal" lines
        if(vertical)
            x0 = A.y
            y0 = A.x
            x1 = B.y
            y1 = B.x

        // Mirror lines going the wrong direction
        if(x0 > x1)
            var/temp = x0
            x0 = x1
            x1 = temp

            temp = y0
            y0 = y1
            y1 = temp

        var
            dx = x1 - x0
            dy = abs(y1 - y0)
            error = 0
            derror = dy / dx
            ystep = y0<y1?1:-1
            y = y0

        var/line[0]
        for(var/x in x0 to x1)
            if(vertical)
                line += locate(y, x, A.z)
            else
                line += locate(x, y, A.z)
            error += derror
            if(error >= 0.5)
                y += ystep
                error -= 1

        return line


I've never used it personally but just glancing at it, it seems to provide what you need.
Mar 23 2014, 11:01 am
Cool! So looking at it, I would type:

get_line(usr,src)
     if(/obj/bglass/ in line) return


Right?
Mar 23 2014, 1:15 pm
In reference to the exact get_line code above, it would be used like so:

proc/found_bullet_blockade(var/atom/point_a, var/atom/point_b)
    var/list/inline_turfs = get_line(point_a, point_b)
    for(var/turf/t in inline_turfs)
        if(t.is_blockade())
            return 1

turf
    proc/is_blockade()
        // assuming your bulletproof glass is an object.
        
        // method 1:
        var/found_bulletproof_glass = locate(/obj/bulletproof_glass) in src
        return found_bulletproof_glass
    
        // method 2:
        for(var/obj/bulletproof_glass/bg in src)
            return 1


This is not the most efficient way of going about this. If that's something you care about, you want to take the premise of what I did and put that inside of its own separate function (check_blockades maybe). This way, you wouldn't be grabbing a bunch of turfs you won't need if there's a blockade closer to you. I'll leave that for you to do if you decide it's a priority, it's pretty simple.
Mar 30 2014, 8:18 am
 
if(blp(src,usr))
                        R.ammo--
                        R.name = "Rifle([R.ammo])"
                        view() << "\red\bold [usr] shoots at [src] with a rifle, but the shot glances off of the glass!"
                        return


This is what I have so far, is this right? It doesn't seem to work, I modified your code a bit for method 2.