Obj loc not being nulled from mob/Cross()

ID:1791274
 
Feb 21 2015, 10:36 am
 
mob/Cross ( skill/_skill )
    .. ( )
    if ( ! ( istype ( src , /mob/npc ) ) )
        if ( istype ( _skill , /skill ) && _skill.damage && _skill.owner )
            take_damage ( _skill.damage , _skill.owner )
            if ( _skill.damage > 50000 && istype ( _skill , /skill/blast ) )
                crater ( loc )
            _skill.remove ( )

skill
    parent_type = /obj

    var
        attack_sound
        damage
        mob/owner


    New ( _damage , mob/_owner )
        dir     = _owner.dir
        loc     = _owner.loc
        owner   = _owner
        damage  = _damage
        density = _owner.density
        move ( )
        //spawn ( 50 )
        //  remove ( )
        . = .. ( ) //As a side-question, is this the proper way to do this


    proc
        move ( )
            walk ( src , dir )
            //spawn ( 50 )
            //  remove ( )

        remove ( )
            if ( src && loc )
                loc = null
                walk ( src , 0 )

It doesn't matter if I spawn remove() in move() or in New(), in mob/Cross() when I call _skill.remove(), it doesn't do anything; it's removed when the spawn calls remove().

That's a problem because I need to remove the skill immediately rather than the 5 seconds or however long after it hits a mob.

When it's called from mob/Cross() it gets to the end of remove(), however the skill is still visible on the map until the spawn calls it. I can simply null the icon_state to hide it until the spawn calls remove() but I'd rather null the location of the skill obj itself immediately, rather than wait.

Am I going about this incorrectly or am I just not understanding why it's acting like it is?
Feb 21 2015, 8:26 pm
First observation: Good gads, so much whitespace!

In New() you don't need to assign ..() to . or alter the return value in any way. Just calling ..() is fine.

In mob/Cross(), calling ..() is meaningless unless you return its value. Your mob/Cross() proc is effectively always returning 0.

Cross() is not actually the correct place to trigger an action. You want Crossed() for that.

I would recommend adding some debug output to see which sections of the code are being reached.
Feb 22 2015, 4:45 am
In response to Lummox JR
Lummox JR wrote:
First observation: Good gads, so much whitespace!
Ya, I think it looks nicer and the same, if not more readability, I guess it's up to personal preference. Whitespace doesn't have a negative performance impact, right?

In New() you don't need to assign ..() to . or alter the return value in any way. Just calling ..() is fine.
Thank you.

In mob/Cross(), calling ..() is meaningless unless you return its value. Your mob/Cross() proc is effectively always returning 0.
Cross() is not actually the correct place to trigger an action. You want Crossed() for that.
Crossed() isn't being triggered in skill/Crossed() or mob/Crossed(), which is why I went for Cross(); would Bump() be better?

skill
    parent_type = /obj

    var
        attack_sound
        damage
        mob/owner


    New ( _damage , mob/_owner )
        dir     = _owner.dir
        loc     = locate ( _owner.x + 1 , _owner.y , _owner.z ) // For testing so it doesn't immediately hit the owner of the skill, I'll later adjust the position based on the dir of the owner
        owner   = _owner
        damage  = _damage
        density = _owner.density
        move ( )
        .. ( )


    Bump ( mob/_mob ) //This doesn't need to be atom/a does it? If I use atom/a, a doesn't get the take_damage proc from mob/proc for some reason, shouldn't atom's inherit (or rather, already have since /mob is a descendant of /atom) all /mob procs?
// I'd assume I would only need for it to be an atom if I want skills to interact with the terrain in some way, which at the moment I don't want
        if ( ! ( istype ( _mob , /mob/npc ) ) )
            if ( istype ( src , /skill ) && damage && owner ) // I probably don't need to istype() src?
                //var mob/_mob = a *** I had this when I used atom/a initially as an argument in Bump()
                _mob.take_damage ( damage , owner )
                if ( damage > 50000 && istype ( src , /skill/blast ) )
                    crater ( loc )
                remove ( )


    proc
        move ( )
            walk ( src , dir )
            spawn ( 50 )
                remove ( )

        remove ( )
            if ( src && loc )
                loc = null
                walk ( src , 0 )

Right now, this code is working exactly how I want skills to function.

I would recommend adding some debug output to see which sections of the code are being reached.
I already did that before I posted this, and when mob/Cross() called _skill.remove(), it reached the end of the remove() proc just fine; it just didn't null the location, however it did end walk().
Feb 22 2015, 10:18 am
In response to Dayoak
Dayoak wrote:
Lummox JR wrote:
First observation: Good gads, so much whitespace!
Ya, I think it looks nicer and the same, if not more readability, I guess it's up to personal preference. Whitespace doesn't have a negative performance impact, right?

It does not impact performance negatively. But bear in mind, ridiculous amounts of whitespace are actually worse for readability than using whitespace in a more reserved manner. I've never understood the compulsion some programmers have to put whitespace inside parentheses, but I hate reading and working with their code.

Crossed() isn't being triggered in skill/Crossed() or mob/Crossed(), which is why I went for Cross(); would Bump() be better?

Bump() would be good if the skill and the mob are both dense. In that case Cross() might be called, but Crossed() definitely would not because the crossing would never occur. (In Bump() you do not need to define the arg as an atom; you can define it as a mob, but you definitely want the istype check there to be sure. If you say the argument is mob/M for instance, and it bumps an obj, M will contain an obj because DM does not do strict type checking.)

However this has me wondering: Are you using pixel movement at all? I'm not certain offhand if cross/uncross get used in traditional tile-based movement.
Feb 22 2015, 12:12 pm
In response to Lummox JR
Lummox JR wrote:
It does not impact performance negatively. But bear in mind, ridiculous amounts of whitespace are actually worse for readability than using whitespace in a more reserved manner.
I've been told/read this before, I'll keep that in mind.

I've never understood the compulsion some programmers have to put whitespace inside parentheses, but I hate reading and working with their code.
I'll probably remove my obsessive amounts of whitespace in future posts.

Bump() would be good if the skill and the mob are both dense. In that case Cross() might be called, but Crossed() definitely would not because the crossing would never occur.
I figured the Crossed() part out, which is why I went with Cross(). However they both will not be dense all the time, which is why I'm now using mob/Crossed() in my code below.

(In Bump() you do not need to define the arg as an atom; you can define it as a mob, but you definitely want the istype check there to be sure. If you say the argument is mob/M for instance, and it bumps an obj, M will contain an obj because DM does not do strict type checking.)
As I thought, thank you.

However this has me wondering: Are you using pixel movement at all? I'm not certain offhand if cross/uncross get used in traditional tile-based movement.
Yes, I'm using pixel movement.

mob/Crossed(skill/_skill)
    if(!density)
        if(istype(_skill, /skill) && !_skill.density && _skill.damage && _skill.owner)
            _skill.hit(src)

skill
    parent_type=/obj

    var
        attack_sound
        damage
        mob/owner


    New(_damage, mob/_owner)
        dir=_owner.dir
        loc=locate(_owner.x, _owner.y, _owner.z)
        owner=_owner
        damage=_damage
        density=_owner.density
        move()
        ..()

    Bump(mob/_mob)
        if(istype(_mob, /mob))
            if(density && damage && owner)
                hit(_mob)

    proc
        move()
            walk(src, dir)
            spawn(50)
                remove()

        remove()
            if(src && loc)
                loc=null
                walk(src, 0)

        hit(mob/_mob)
            if(!(istype(src, /mob/npc)))
                _mob.take_damage(damage, owner)
                if(damage>50000 && istype(src, /skill/blast))
                    crater(loc)
                remove()
Feb 22 2015, 6:09 pm
One other item worth considering: If the mob moves before the projectile does, the mob can cross onto the projectile first. All non-dense projectile systems have to account for this.
Feb 23 2015, 8:45 am
In response to Lummox JR
Lummox JR wrote:
One other item worth considering: If the mob moves before the projectile does, the mob can cross onto the projectile first. All non-dense projectile systems have to account for this.

So basically if I understand what you mean, I have to put another Crossed() in /skill for a skill to hit if a mob crosses a skill, since I only have it set up for a skill crossing/bumping a mob.

skill/Crossed(mob/_mob)
    if(istype(_mob, /mob) && !density)
        if(!_mob.density && damage && owner)
            hit(_mob)