Bump() not called consistently

ID:2003375
 
Dec 22 2015, 1:48 pm
Descriptive Problem Summary: When one mob attempts to move into another (for example, as the result of gravity pushing one mob against another), Bump() is not called every tick

Numbered Steps to Reproduce Problem:
in a while-true loop, sleeping every world.tick_lag, have one mob attempt to pixel-move into another mob below it, that is not part of its group

Code Snippet (if applicable) to Reproduce Problem:
while (1)
    for(var/atom/A in world)
        A.Tick()
    sleep(world.tick_lag)



/atom/movable/proc/MoveBy(var/StepX, var/StepY)
    var/NewX = (StepX + step_x + SubStepX) + (x * world.icon_size)
    var/NewY = (StepY + step_y + SubStepY) + (y * world.icon_size)

    SubStepX = NewX - round(NewX)
    SubStepY = NewY - round(NewY)

    return Move(locate(round(NewX / world.icon_size), round(NewY / world.icon_size), z), 0, round(NewX % world.icon_size), round(NewY % world.icon_size))


Bump(atom/movable/AM)
    . = ..()
    world.log << "Bump, [src] against [AM]"
    if (YVelocity < 0 && ismob(AM))
        AM:Riders += src

Tick()
    . = ..()

    if (XVelocity < -MaximumVelocity)
        XVelocity = -MaximumVelocity
    if (XVelocity > MaximumVelocity)
        XVelocity = MaximumVelocity
    if (YVelocity > MaximumVelocity)
            YVelocity = MaximumVelocity

    YVelocity -= (Gravity * GetGravityModifier())
    if (YVelocity < -MaximumVelocity)
        YVelocity = -MaximumVelocity

    for(var/atom/movable/AM in Riders)
        AM.MoveBy(XVelocity, 0)
        if (YVelocity > 0 || (StickyPlatform && YVelocity != 0))
            AM.MoveBy(0, YVelocity)

    Riders = list( )

    if (!MoveBy(XVelocity, 0))
        XVelocity = 0 // 0 Velocity if we hit a wall going sideways

    if (YVelocity < 0)
        Grounded = !MoveBy(0, YVelocity) // Try to move downwards, and flag Grounded if we cant (i.e. riding a mob, solid floor, etc)
        if (Grounded)
            YVelocity = 0
    else
        Grounded = FALSE
        // Try to move upwards, and 0 velocity if we hit a ceiling
        if (!MoveBy(0, YVelocity))
            YVelocity = 0


Expected Results:
Bump() to be called every tick

Actual Results:
Bump() called every other tick

Does the problem occur:
Every time? Or how often? Every time
In other games? Not tested
In other user accounts? Not tested
On other computers? Yes

When does the problem NOT occur? If a /mob is bumping against an /obj, Bump() is called consistently every tick

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? Not tested

Workarounds: Workaround not found
Dec 22 2015, 1:58 pm
I have a (small) project that demonstrates this
Dec 22 2015, 2:05 pm
I find it very, very difficult to believe this is a bug rather than a code problem. Physics systems are notoriously tricky. You should add some debugging to your Move() so you can see what's being called and where, and what the default Move() is returning.

I also see a flaw in your implementation right off the bat: In MoveBy(), you're not accounting for cases where Move() returns partial movement, in which case you'd want either SubStepX or SubStepY, or both, to be reset. Another flaw is that the multiplication, division, and modulo you're doing with each is unnecessary.
Dec 22 2015, 2:37 pm
Okay yeah. I owe you an apology for this one. SubStepY was the problem; because of that oversight on my part with respect to setting it without checking, it was looping around until it would get to 0.81 and the next tick the player wouldn't move far down enough to actually trigger this.

Well, there's a bit of pie on my face.
Dec 22 2015, 3:15 pm
Nadrew resolved issue (Not a bug)
Dec 22 2015, 3:42 pm
No problem; sometimes a misbehaving piece of code really looks like a bug, and sometimes a bug really looks like a misbehaving piece of code.
Dec 22 2015, 4:04 pm
BTW, this is how I'd modify MoveBy:

atom/movable/proc/MoveBy(StepX, StepY)
    var/sx = step_x, sy = step_y    // put these in local vars for faster access
    var/ssx = SubStepX, ssy = SubStepY  // same deal
    var/NewSX = (StepX + sx + ssx)
    var/NewSY = (StepY + sy + ssy)
    var/RX = round(NewSX,1), RY = round(NewSY,1)
    var/dx = NewSX - sx, dy = NewSY - sy
    var/ax = abs(RX-sx), ay = abs(RY-sy)

    SubStepX = NewSX - RX
    SubStepY = NewSY - RY
    . = Move(loc, 0, RX, RY)

    if(. < max(ax,ay))
        if(ax > ay)
            SubStepX = 0
            // apply fractional Y change
            ssy = dy * . / ax
            SubStepY = ssy - round(ssy, 1)
        else if(ay > ax)
            SubStepY = 0
            // apply fractional X change
            ssx = dx * . / ay
            SubStepX = ssx - round(ssx, 1)
        else
            SubStepX = 0
            SubStepY = 0

I used local vars liberally because they're a crapton faster. This looks at the movement that was intended, and if the Move() result is less than the maximum step_x/y change, it knows something got bumped. At that point it tries to figure out the predominant movement direction, and resets the sub-step for that direction; the other one is adjusted based on how far it got. So if you're moving 4 pixels east and 3 north (in actual pixels, after your existing sub-step is taken into account), and Move() returns 3, it knows you only moved 3/4 of the way. That represents 3 pixels east, 2.25 north, and therefore SubStepY ends up being 0.25 after rounding.
Dec 30 2015, 3:48 pm
Hey, I appreciate it. Having more-accurate movement code is always good.