Is there a better way to do this?

ID:2347958
 
Feb 22 2018, 2:07 pm
(See the best response by Kaiochao.)
Code:
obj/items
    bombs
        icon = 'bomb.dmi'
        icon_state = "inv"
        layer = MOB_LAYER+0.5
        density = 1
        var/shaking = 0
        proc
            bshake()
                if(!shaking)
                    shaking = 1
                    spawn()
                        while(src.loc != null)
                            sleep(1)
                            shake(1)
                            if(loc == null)break
            explode()
                var/obj/explosion/c = new(loc)
                var/obj/explosion/n = new(locate(x,y+1,z))
                var/obj/explosion/s = new(locate(x,y-1,z))
                var/obj/explosion/w = new(locate(x-1,y,z))
                var/obj/explosion/e = new(locate(x+1,y,z))
                var/list/explosions = list(c,n,s,w,e)
                if(Owner.bomb_radius >= 2)
                    var/obj/explosion/n2 = new(locate(x,y+2,z))
                    var/obj/explosion/s2 = new(locate(x,y-2,z))
                    var/obj/explosion/w2 = new(locate(x-2,y,z))
                    var/obj/explosion/e2 = new(locate(x+2,y,z))
                    explosions.Add(n2,s2,w2,e2)
                if(Owner.bomb_radius >= 3)
                    var/obj/explosion/n3 = new(locate(x,y+3,z))
                    var/obj/explosion/s3 = new(locate(x,y-3,z))
                    var/obj/explosion/w3 = new(locate(x-3,y,z))
                    var/obj/explosion/e3 = new(locate(x+3,y,z))
                    explosions.Add(n3,s3,w3,e3)
                if(Owner.bomb_radius >= 4)
                    var/obj/explosion/n4 = new(locate(x,y+4,z))
                    var/obj/explosion/s4 = new(locate(x,y-4,z))
                    var/obj/explosion/w4 = new(locate(x-4,y,z))
                    var/obj/explosion/e4 = new(locate(x+4,y,z))
                    explosions.Add(n4,s4,w4,e4)
                if(Owner.bomb_radius >= 5)
                    var/obj/explosion/n5 = new(locate(x,y+5,z))
                    var/obj/explosion/s5 = new(locate(x,y-5,z))
                    var/obj/explosion/w5 = new(locate(x-5,y,z))
                    var/obj/explosion/e5 = new(locate(x+5,y,z))
                    explosions.Add(n5,s5,w5,e5)
                if(Owner.bomb_radius == 6)
                    var/obj/explosion/n6 = new(locate(x,y+6,z))
                    var/obj/explosion/s6 = new(locate(x,y-6,z))
                    var/obj/explosion/w6 = new(locate(x-6,y,z))
                    var/obj/explosion/e6 = new(locate(x+6,y,z))
                    explosions.Add(n6,s6,w6,e6)
                for(var/obj/E in explosions)
                    E.Owner = src
                for(var/obj/A in explosions)
                    for(var/obj/crates/unbreakable_box/b)
                        var/turf/Location = b.loc
                        if(A.loc == Location)
                            b.shake(3)
                            A.loc = null
                loc = null


Problem description: I need help with my explode code I need the explosions to stop when they reach the "Unbreakable Boxes" With my code the explosion only stops at the location where the box is place but doesnt stop the explosions after.

Example of what I mean:

Feb 22 2018, 2:17 pm
You could do it with nested for() loops over each cardinal direction and distance from 1 to the radius. If you reach an unbreakable box, simply break out of the distance loop.
Feb 22 2018, 2:33 pm
In response to Kaiochao
Kaiochao wrote:
You could do it with nested for() loops over each cardinal direction and distance from 1 to the radius. If you reach an unbreakable box, simply break out of the distance loop.

is nested for() loops covered in the DM guide?
Feb 22 2018, 4:19 pm
I don't know if nested for loops are covered specifically in the guide, but for loops are. And once you get to know how to use a for loop, using a nested for comes naturally - in fact, you already have one in your code: 
// Loops through all explosions
for(var/obj/A in explosions)
    // Then loop through EVERY UNBREAKABLE BOX IN THE WORLD
    // and check to see if it has the same location
    for(var/obj/crates/unbreakable_box/b)
        var/turf/Location = b.loc
            if(A.loc == Location)
                b.shake(3)
                A.loc = null

This is a bit of a digression, but an important one. There are more ways to reference an object than to loop through all of some object type in some container. The easiest way to have done that would have been: 
// Loop through all explosions
for(var/obj/A in explosions)
    // Check for unbreakable crates in the same loc
    var/obj/crates/unbreakable_box/same_loc_box = locate() in A.loc
    if(same_loc_box)
        same_loc_box.shake(3)
        A.loc = null

One of the most important uses of a for loop is to iterate over an index. In other words, you can do something 1 to 10 times, for example. This is great for you because you can create an explosion for each step from 1 to bomb_radius. You can then combine (nest) this with a loop over each direction: 
var/list/cardinal_directions = list(EAST, NORTH, WEST, SOUTH)
for(var/explode_direction in cardinal_directions)
    for(var/step_index = 1 to Owner.bomb_radius)
        do something

Now at each step (where it says "do something") we know what direction we're exploding in, and how far away (step_index) we are from the center. You can also use the break keyword to stop moving outward if you encounter an unbreakable block. 
explode()
    var/cardinal_directions = list(EAST, NORTH, WEST, SOUTH)
    for(var/explode_direction in cardinal_directions)
        var/turf/current_step = loc
        for(var/step_distance = 1 to Owner.bomb_radius)
            current_step = get_step(current_step, explode_direction)
            var/obj/crates/unbreakable_box/explode_stopper = locate() in current_step
            if(explode_stopper)
                explode_stopper.shake(3)
                break
            var/obj/explosion/new_explosion = new(current_step)
            new_explosion.Owner = src
    loc = null

That might not make a lot of sense, which is why you should always write comments as you write code. The comments help you to understand WHY you're doing something, not just WHAT you're doing. You'll write much better code if you write comments as you go, and you'll also be able to get help much more easily on the forums (because people understand the PURPOSE of your code, which isn't always obvious when the code is broken): 
explode()
    // Loop through each of the cardinal directions, creating explosions up to bomb_radius
    var/cardinal_directions = list(EAST, NORTH, WEST, SOUTH)
    for(var/explode_direction in cardinal_directions)
    // Loop from 1 to bomb radius, stepping forward each
        var/turf/current_step = loc
        for(var/step_distance = 1 to Owner.bomb_radius)
            current_step = get_step(current_step, explode_direction)
    // Check for unbreakable crates at this step. If found, stop stepping in this direction.
    // Also, shake it.
            var/obj/crates/unbreakable_box/explode_stopper = locate() in current_step
            if(explode_stopper)
                explode_stopper.shake(3)
                break
    // Create the explosion at each step, and set its owner
            var/obj/explosion/new_explosion = new(current_step)
            new_explosion.Owner = src
    // Remove bomb from the map
    loc = null
Feb 22 2018, 5:07 pm
In response to IainPeregrine
Best response
Thanks for the detailed explanation... but I wrote pretty much the exact same code in the Discord a couple hours ago :X
// Create the central explosion.

// Create the extended explosions.
// The explosion extends in every direction.
for(var/direction in list(NORTH, SOUTH, EAST, WEST))
  // The explosion extends a certain distance away until it's interrupted.
  var turf/location = loc
  for(var/distance = 1 to bomb_radius)
    location = get_step(location, direction)

    // Shake any boxes at the current location.
    var box_hit = FALSE
    for(var/obj/crates/unbreakable_box/box in location)
      box_hit = TRUE
      box.shake(3)

    // If any boxes were hit, then stop extending the explosion in this direction.
    if(box_hit)
      break
    else
      // Make explosion at the current location.

I left out instantiation to explain how he could include the owner as a constructor argument.
Feb 22 2018, 5:32 pm
Yes thank you both very helpful I appreciate it.
Feb 22 2018, 6:48 pm
In response to Kaiochao
Kaiochao wrote:
Thanks for the detailed explanation... but I wrote pretty much the exact same code in the Discord a couple hours ago :X

Didn't mean to crowd out your advice. I've been haunting the dev-help forum as a way to re-acclimate myself to DM.

I left out instantiation to explain how he could include the owner as a constructor argument.

I wrote up a "how would I do this" version that passed the owner as an argument - so, yeah, thinking pretty similar. I also added two procs to turf: breakable() and break(). Breakable returns TRUE all the contents are breakable, so there can be multiple different kinds of unbreakable blocks, or so some blocks are selectively breakable to different kinds of bombs. break() does the actual work of breaking the contents.

All in all I've had too much time on my hands lately.
Feb 22 2018, 10:48 pm (Edited on Feb 22 2018, 11:07 pm)
This is probably in line with Kaio's solution, but there's a few things he's doing that kind of mask understanding of how DM works under the hood.

Key thing is that you are writing a lot of logic in the wrong places. By not embedding the logic in the objects you are creating, you are actually inducing extra overhead by maintaining and looping through a list over and over again. It's completely not necessary if you just rearrange the order of the steps to be in line with the creation. This requires you to split your code up into multiple places, but overall it reduces the cost of your design and the amount of code you have to write.

Since you are embedding the logic in other objects, it may actually save you time down the road to make the behavior more modular by embedding it at a higher level than strictly necessary. By doing this, you can reduce the amount of specific code you need to write per-object, and actually wind up being able to implement a variety of behavior in just one or two lines of code.

obj/items
    bomb
        icon = 'bomb.dmi'
        icon_state = "inv"
        layer = MOB_LAYER+0.5
        density = 1
        var
            shaking = 0
        proc
            bshake()
                set waitfor = 0
                if(shaking) return
                while(loc)
                    sleep(1)
                    shake(1)

            explode()
                var/obj/explosion/c = new/obj/explosion(loc,src)
                loc = null
                if(c.loc)
                     var/turf/t, dir, dist, obj/explosion/e
                    //loop over the 4 cardinal directions. 1 = NORTH, 2 = SOUTH, 4 = EAST, 8 = WEST. Thus, we don't need to create a list or deal with the overhead of reading from it. We can just straight up use a number and double it.
                    for(dir=1; dir<=8; dir*=2)
                        e = c
                        for(dist=bomb_radius; dist; dist--) //loop over a line of turfs in direction until distance is reached.
                            t = get_step(e.loc,dir)
                            if(!t) break //early ejection if no turf is found (edge of the map), or the explosion was blocked somehow.
                            e = new/obj/explosion(t,src)



//add top-level explosion properties and procs.
atom
    var
        explosion_density = 0
    proc
        //called to determine if an object allows an explosion to affect the object.
        //return 1 to allow the explosion to exist on the same tile.
        //return 0 to block the explosion from existing on this tile.

        Explode(obj/explosion/e)
            return !explosion_density

        //called when an explosion has affected the object.
        Exploded(obj/explosion/e)

        //called when an explosion fails to affect the object.
        BlockExplosion(obj/explosion/e)

obj/explosion
    var
        obj/owner

    //we're letting explosions sort of handle their own logic in New().
    New(atom/loc,obj/owner)
        src.owner = owner
        if(src.loc)
            //loop over the location's contents backward and call the explosion hooks.
            var/list/l = loc + loc.contents - src, len = length(l), atom/o
            for(var/pos in len to 1 step -1)
                o = l[pos]
                if(!o.Explode(src))
                    src.loc = null
                    o.BlockExplosion(src) //if something caused this explosion to fail, we should call the blockexplosion hook.
                    break
            if(src.loc)
                //if nothing caused this explosion to fail, we should call the exploded hook.
                for(var/pos in len to 1 step -1)
                    o = l[pos]
                    o.Exploded(src)

//an example of how to make something block an explosion.

obj/crates/unbreakable
    explosion_density = 1

    BlockExplosion(obj/explosion/e)
        shake(3)

//just an example of how to make crates able to blow up.

obj/crates
    Exploded(obj/explosion/e)
        shake(1)
        loc = null
Feb 22 2018, 10:52 pm
In response to Ter13
I can't really understand whats going on in your for() loops
Feb 22 2018, 11:05 pm
 
1 for(dir=1; dir<=8; dir*=2)
2     e = c
3     for(dist=bomb_radius; dist; dist--)
4         t = get_step(e.loc,dir)
5         if(!t) break
6         e = new/obj/explosion(t,src)


1) outer for loop, iterates, setting value to 1 initially, then doubles it every loop while it is less or equal to 8.
pattern: [1, 2, 4, 8] (NORTH, SOUTH, EAST, WEST)

2) assign the variable e to c. "e" is the current explosion, "c" is the central explosion.

3) inner for loop, iterates setting value to the radius of the bomb initially, then decreases it by 1 until reaching 0.

4) Get the turf t along the line in direction dir from the last explosion created (or the center for the first iteration.)

5) if there is no turf, break out of the inner loop and return to the outer loop.

6) Create a new explosion at t, and pass src as the owner argument for obj/explosion/New().

1 for(var/pos in len to 1 step -1)
2     o = l[pos]
3     if(!o.Explode(src))
4         src.loc = null
5         o.BlockExplosion(src)
6         break


1) set a for loop to count variable pos down from variable len to 1. len is the length of the contents list for the current location excluding the explosion. ex: [5, 4, 3, 2, 1]

2) get the object at list index pos and assign it to variable o.

3) if the object's Explode() proc returns 0.

4) set the loc of the explosion to null.

5) tell the object it blocked this explosion.

6) terminate the loop early.

The last loop is pretty much the same, but it calls Explode() on everything in l.