Code cleanup

Clean up and improve the between-actors collision code:

* Give names to the A, B tuples from BetweenBoxes and call them the stable and
  mover doodads. All ops are from the perspective of the stable (A) box in
  relation to the mover (B).
* Replace the map[*Actor]*Actor collision struct to one that supports arrays of
  colliding actors, enabling one actor to post OnLeave events correctly when it
  overlapped several actors at once.
* Possibly improve on some of the collision bugs along the way.

Some issues remaining:

* If playing AS an offset-hitbox character:
    * Solid hitboxes against closed doors and platforms works OK.
    * Button InHitbox (overlap) checks do not fire (buttons won't press).
* If playing with a non-offset character (e.g. boy.doodad)
    * Solid hitboxes work and Buttons will press correctly.
* Both types of player hitboxes:
    * When touching a static object such as the Start Flag which has an offset
      hitbox of its own: the Start Flag will creep up the level on its Y axis
      when touched before it settles and won't move.

pkg/balance/numbers.go

@@ -57,6 +57,9 @@ var (
 	CoyoteFrames            uint64  = 4  // Coyote time, frames after we walk off a cliff but can still jump late
 	SlopeMaxHeight                  = 8  // max pixel height for player to walk up a slope
 
+	// Collision detection threshold to consider an actor "on top" of a doodad's solid hitbox.
+	OnTopThreshold = 4
+
 	// Number of game ticks to insist the canvas follows the player at the start
 	// of a level - to overcome Anvils settling into their starting positions so
 	// they don't steal the camera focus straight away.

pkg/collision/bounding_rect.go

@@ -24,7 +24,9 @@ func GetBoundingRect(a Actor) render.Rect {
 // account their self-declared collision hitbox.
 //
 // The rect returned has the X,Y coordinate set to the actor's position, plus
-// the X,Y of their hitbox, if any.
+// the X,Y of their hitbox, if any. For example, their sprite size could be 64x32
+// and their hitbox is the lower 0,32,32,32 half. This function would return the
+// world coordinate of where their bounding box begins.
 //
 // The W,H of the rect is the W,H of their declared hitbox.
 //

pkg/collision/collide_level.go

@@ -117,7 +117,7 @@ func BoxCollidesWithGrid(d Actor, grid *level.Chunker, target render.Point) (*Co
 	// Adjust the actor's bounding rect by its stated Hitbox from its script.
 	// e.g.: Boy's Canvas size is 56x56 but he is a narrower character with a
 	// hitbox width smaller than its Canvas size.
-	S = SizePlusHitbox(GetBoundingRect(d), hitbox)
+	S = GetBoundingRectHitbox(d, hitbox)
 	actorHeight := P.Y + S.H
 
 	// Test if we are ALREADY colliding with level geometry and try and wiggle

pkg/fps.go

@@ -160,11 +160,10 @@ func (d *Doodle) DrawCollisionBox(canvas *uix.Canvas, actor *uix.Actor) {
 	var (
 		rect = collision.GetBoundingRect(actor)
 		box  = collision.GetCollisionBox(rect)
-		hitbox = actor.Hitbox()
 	)
 
 	// Adjust the actor's bounding rect by its stated Hitbox from its script.
-	rect = collision.SizePlusHitbox(rect, hitbox)
+	rect = collision.GetBoundingRectHitbox(actor, actor.Hitbox())
 
 	box = collision.GetCollisionBox(rect)
 

pkg/uix/actor_collision.go

@@ -31,6 +31,7 @@ func (w *Canvas) loopActorCollision() error {
 		// collision later, store each actor's original position before the move.
 		boxes             = make([]render.Rect, len(w.actors))
 		originalPositions = map[string]render.Point{}
+		originalHitboxes  = map[string]render.Rect{} // original world hitboxes
 	)
 
 	// Loop over all the actors in parallel, processing their movement and
@@ -48,8 +49,10 @@ func (w *Canvas) loopActorCollision() error {
 		func(i int, a *Actor) {
 			// defer wg.Done()
 			originalPositions[a.ID()] = a.Position()
+			originalHitboxes[a.ID()] = collision.GetBoundingRectHitbox(a, a.Hitbox())
 
 			// Advance any animations for this actor.
+			// TODO: wallclock time here, should be set by FPS for consistency.
 			if a.activeAnimation != nil && a.activeAnimation.nextFrameAt.Before(now) {
 				if done := a.TickAnimation(a.activeAnimation); done {
 					// Animation has finished, get the callback function.
@@ -130,31 +133,42 @@ func (w *Canvas) loopActorCollision() error {
 			w.loopContainActorsInsideLevel(a)
 
 			// Store this actor's bounding box after they've moved.
-			boxes[i] = collision.SizePlusHitbox(collision.GetBoundingRect(a), a.Hitbox())
+			boxes[i] = collision.GetBoundingRect(a)
 		}(i, a)
 		// wg.Wait()
 	}
 
-	var collidingActors = map[*Actor]*Actor{}
+	// log.Warn("== BEGIN BetweenBoxes")
+
+	// Check pairs of all our Actor boxes for overlap and running their OnCollide
+	// scripts for mobile actors.
+	var collidingActors = ActorCollisionMap{}
 	for tuple := range collision.BetweenBoxes(boxes) {
-		a, b := w.actors[tuple.A], w.actors[tuple.B]
+
+		// Give the A, B tuple of boxes names: their order doesn't matter.
+		// Example: stable could be the Button and mover is the Player walking onto it.
+		// Or: stable could be the Player and mover is a Key that they walked onto.
+		stable, mover := w.actors[tuple.A], w.actors[tuple.B]
 
 		// If neither actor is mobile, don't run collision handlers.
-		if !(a.IsMobile() || b.IsMobile()) {
+		if !(stable.IsMobile() || mover.IsMobile()) {
 			continue
 		}
 
-		collidingActors[a] = b
+		collidingActors.Set(stable, mover)
 
-		// log.Error("between boxes: %+v  <%s> <%s>", tuple, a.ID(), b.ID())
+		log.Error("between boxes: %+v  A=<%s>  B=<%s>", tuple, stable.ID(), mover.ID())
 
 		// Call the OnCollide handler for A informing them of B's intersection.
 		if w.scripting != nil {
 			var (
-				rect        = collision.SizePlusHitbox(collision.GetBoundingRect(b), b.Hitbox())
+				rect = collision.GetBoundingRectHitbox(mover, mover.Hitbox())
+				// lastGoodBox = rect
 				lastGoodBox = render.Rect{
-					X: originalPositions[b.ID()].X,
+					// Level Positions of the doodad is based on the top left
-					Y: originalPositions[b.ID()].Y,
+					// of its graphical sprite, not its (possibly offset) hitbox.
+					X: originalPositions[mover.ID()].X,
+					Y: originalPositions[mover.ID()].Y,
 					W: boxes[tuple.B].W,
 					H: boxes[tuple.B].H,
 				}
@@ -171,7 +185,7 @@ func (w *Canvas) loopActorCollision() error {
 			// use it for collision-check purposes but DON'T physically move
 			// the character by it (moving the character may clip them thru
 			// other solid hitboxes like the upside-down trapdoor)
-			var onTopY int
+			// var onTopY int
 
 			// Firstly we want to make sure B isn't able to clip through A's
 			// solid hitbox if A protests the movement. Trace a vector from
@@ -180,27 +194,40 @@ func (w *Canvas) loopActorCollision() error {
 			// only return false if it protests the movement, but not trigger
 			// any actions (such as emit messages to linked doodads) until
 			// Settled=true.
-			if origPoint, ok := originalPositions[b.ID()]; ok {
+			if origHitbox, ok := originalHitboxes[mover.ID()]; ok {
-				// Trace a vector back from the actor's current position
-				// to where they originated from. If A protests B's position at
-				// ANY time, we mark didProtest=true and continue backscanning
-				// B's movement. The next time A does NOT protest, that is to be
-				// B's new position.
 
+				var (
 					// Special case for when a mobile actor lands ON TOP OF a solid
 					// actor. We want to stop their Y movement downwards, but allow
 					// horizontal movement on the X axis.
 					// Touching the solid actor from the side is already fine.
-				var onTop = false
+					onTop    bool
+					onBottom bool // they hit the bottom instead
+					// onSide   bool // they hit a side, maybe allow Y movement
 
-				var (
+					// If we lock their movement coordinate.
-					lockX int
+					lockX *int
-					lockY int
+					lockY *int
 				)
 
+				// If their original hitbox is offset from their sprite corner,
+				// gather the offset now.
+				var (
+					origPosition  = originalPositions[mover.ID()]
+					hitboxPadding = render.Point{
+						X: render.AbsInt(origHitbox.X - origPosition.X),
+						Y: render.AbsInt(origHitbox.Y - origPosition.Y),
+					}
+				)
+
+				// Trace a vector back from the mover's current position
+				// to where they originated from. If A protests B's position at
+				// ANY time, we ?mark didProtest=true? and continue backscanning
+				// B's movement. The next time A does NOT protest, that is to be
+				// B's new position.
 				for point := range render.IterLine(
-					origPoint,
+					origHitbox.Point(),
-					b.Position(),
+					mover.Position(), // TODO: verify non 0,0 hitbox doodads work
 				) {
 					point := point
 					test := render.Rect{
@@ -211,49 +238,92 @@ func (w *Canvas) loopActorCollision() error {
 					}
 
 					if info, err := collision.CompareBoxes(boxes[tuple.A], test); err == nil {
+						// A and B have their drawings overlapping on the page. Get each
+						// of their declared hitboxes (if smaller) to see if their hitboxes
+						// intersect as well.
+						var (
+							stableHitbox = collision.GetBoundingRectHitbox(stable, stable.Hitbox())
+							moverHitbox  = collision.GetBoundingRectHitbox(mover, mover.Hitbox())
+						)
+
 						// B is overlapping A's box, call its OnCollide handler
 						// with Settled=false and see if it protests the overlap.
-						err := w.scripting.To(a.ID()).Events.RunCollide(&CollideEvent{
+						err := w.scripting.To(stable.ID()).Events.RunCollide(&CollideEvent{
-							Actor:    b,
+							Actor:    mover,
 							Overlap:  info.Overlap,
-							InHitbox: info.Overlap.Intersects(a.Hitbox()),
+							InHitbox: stableHitbox.Intersects(moverHitbox),
 							Settled:  false,
 						})
 
+						// log.Warn("ActorCollision: CompareBoxes info was %+v", info)
+
 						// Did A protest?
 						if err == scripting.ErrReturnFalse {
 							// Are they on top?
-							aHitbox := collision.SizePlusHitbox(collision.GetBoundingRect(a), a.Hitbox())
+							var (
-							if render.AbsInt(test.Y+test.H-aHitbox.Y) == 0 {
+								stableTop    = stableHitbox.Y
-								// log.Error("ActorCollision: onTop=true at Y=%s", test.Y)
+								stableBottom = stableHitbox.Y + stableHitbox.H
+								moverTop     = test.Y
+								moverBottom  = test.Y + test.H // bottom of falling actor
+							)
+
+							// Is the colliding actor on top? (e.g. mover=player character)
+							if render.AbsInt(moverBottom-stableTop) < balance.OnTopThreshold {
 								onTop = true
-								onTopY = test.Y
+								// onTopY = stableHitbox.Y
+							}
+
+							// Or are they hitting from below?
+							if render.AbsInt(stableBottom-moverTop) < balance.OnTopThreshold {
+								onBottom = true
+							}
+
+							if onTop || onBottom {
+								log.Error("onTop=%+v onBottom=%+v", onTop, onBottom)
 							}
 
 							// What direction were we moving?
 							if test.Y != lastGoodBox.Y {
-								if lockY == 0 {
+
-									lockY = lastGoodBox.Y
+								// If we are hitting the top or bottom, lock our Y coordinate here.
+								if onTop || onBottom {
+
+									// First Y coordinate before the protested collision.
+									if lockY == nil {
+										lockY = new(int)
+										*lockY = lastGoodBox.Y
+										if onBottom {
+											*lockY -= hitboxPadding.Y
 										}
+									}
+
+									// If on top, set the mover to Grounded here.
 									if onTop {
-									// log.Error("ActorCollision: setGrounded(true)", test.Y)
+										mover.SetGrounded(true)
-									b.SetGrounded(true)
 									}
 								}
+
+							}
 							if test.X != lastGoodBox.X {
-								if !onTop {
+								if lockX == nil && !(onTop || onBottom) {
-									lockX = lastGoodBox.X
+									lockX = new(int)
+									*lockX = lastGoodBox.X
 								}
 							}
 
 							// Move them back to the last good box.
-							lastGoodBox = test
+							lastGoodBox = render.Rect{
-							if lockX != 0 {
+								X: test.X, // - hitboxPadding.X, // note: this is in World Coordinates
-								lastGoodBox.X = lockX
+								Y: test.Y, // - hitboxPadding.Y,
+								W: test.W,
+								H: test.H,
+							}
+							if lockX != nil {
+								lastGoodBox.X = *lockX - hitboxPadding.X
 							}
 						} else {
 							if err != nil {
-								log.Error("RunCollide on %s (%s) errored: %s", a.ID(), a.Actor.Filename, err)
+								log.Error("RunCollide on %s (%s) errored: %s", stable.ID(), stable.Actor.Filename, err)
 							}
 							// Move them back to the last good box.
 							lastGoodBox = test
@@ -265,65 +335,107 @@ func (w *Canvas) loopActorCollision() error {
 				}
 
 				// Did we lock their X or Y coordinate from moving further?
-				if lockY != 0 {
+				if lockY != nil {
-					lastGoodBox.Y = lockY
+					lastGoodBox.Y = *lockY
 				}
-				if lockX != 0 {
+				if lockX != nil {
-					lastGoodBox.X = lockX
+					lastGoodBox.X = *lockX
 				}
 
-				if !b.noclip {
+				if !mover.noclip {
-					b.MoveTo(lastGoodBox.Point())
+					log.Error("Move B to: %s", lastGoodBox.Point())
+
+					// The stationary doodad should move the moving one only.
+					mover.MoveTo(lastGoodBox.Point())
 				}
 			} else {
 				log.Error(
 					"ERROR: Actors %s and %s overlap and the script returned false,"+
 						"but I didn't store %s original position earlier??",
-					a.Doodad().Title, b.Doodad().Title, b.Doodad().Title,
+					stable.Doodad().Title, mover.Doodad().Title, mover.Doodad().Title,
 				)
 			}
 
-			if onTopY != 0 && lastGoodBox.Y-onTopY <= 1 {
+			// TODO: onTopY != nil
-				lastGoodBox.Y = onTopY
+			// if onTopY != 0 && lastGoodBox.Y-onTopY <= 1 {
-			}
+			// lastGoodBox.Y = onTopY
+			// }
 
 			// Movement has been settled. Check if B's point is still invading
 			// A's box and call its OnCollide handler one last time in
 			// Settled=true mode so it can run its actions.
 			if info, err := collision.CompareBoxes(boxes[tuple.A], lastGoodBox); err == nil {
-				if err := w.scripting.To(a.ID()).Events.RunCollide(&CollideEvent{
+				if err := w.scripting.To(stable.ID()).Events.RunCollide(&CollideEvent{
-					Actor:    b,
+					Actor:    mover,
 					Overlap:  info.Overlap,
-					InHitbox: info.Overlap.Intersects(a.Hitbox()),
+					InHitbox: info.Overlap.Intersects(stable.Hitbox()),
 					Settled:  true,
 				}); err != nil && err != scripting.ErrReturnFalse {
-					log.Error("VM(%s).RunCollide: %s", a.ID(), err.Error())
+					log.Error("VM(%s).RunCollide: %s", stable.ID(), err.Error())
 				}
 
 				// If the (player) is pressing the Use key, call the colliding
 				// actor's OnUse event.
-				if b.flagUsing {
+				if mover.flagUsing {
-					if err := w.scripting.To(a.ID()).Events.RunUse(&UseEvent{
+					if err := w.scripting.To(stable.ID()).Events.RunUse(&UseEvent{
-						Actor: b,
+						Actor: mover,
 					}); err != nil {
-						log.Error("VM(%s).RunUse: %s", a.ID(), err.Error())
+						log.Error("VM(%s).RunUse: %s", stable.ID(), err.Error())
 					}
 				}
 			}
 		}
 	}
 
+	log.Warn("-- END BetweenBoxes")
+
 	// Check for lacks of collisions since last frame.
-	for sourceActor, targetActor := range w.collidingActors {
+	// Note: w.collidingActors is "last frame's" map of colliding actor boxes.
-		if _, ok := collidingActors[sourceActor]; !ok {
+	w.collidingActors.Iter(func(stable, mover *Actor) {
-			w.scripting.To(sourceActor.ID()).Events.RunLeave(&CollideEvent{
+
-				Actor:   targetActor,
+		// Are these not colliding this frame?
+		// TODO: does this work with three-way actor collisions?
+		if !collidingActors.Exists(stable, mover) {
+			w.scripting.To(stable.ID()).Events.RunLeave(&CollideEvent{
+				Actor:   mover,
 				Settled: true,
 			})
 		}
-	}
+	})
 
 	// Store this frame's colliding actors for next frame.
 	w.collidingActors = collidingActors
 	return nil
 }
+
+// ActorCollisionMap keeps a cache of collision box overlaps between
+// an Actor and one or more other Actors.
+type ActorCollisionMap map[*Actor]map[*Actor]interface{}
+
+// Set a collision to the other actor.
+func (m ActorCollisionMap) Set(stable, mover *Actor) {
+	if m[stable] == nil {
+		m[stable] = map[*Actor]interface{}{}
+	}
+
+	m[stable][mover] = nil
+}
+
+// Exists checks if the actor is colliding with the other.
+func (m ActorCollisionMap) Exists(stable, mover *Actor) bool {
+	if m[stable] == nil {
+		return false
+	}
+
+	_, ok := m[stable][mover]
+	return ok
+}
+
+// Iter the collision data.
+func (m ActorCollisionMap) Iter(fn func(stable, mover *Actor)) {
+	for stable, moverMap := range m {
+		for mover := range moverMap {
+			fn(stable, mover)
+		}
+	}
+}

pkg/uix/canvas.go

@@ -90,7 +90,7 @@ type Canvas struct {
 	actors []*Actor // if this canvas CONTAINS actors (i.e., is a level)
 
 	// Collision memory for the actors.
-	collidingActors map[*Actor]*Actor // mapping their IDs to each other
+	collidingActors ActorCollisionMap // mapping their IDs to each other
 
 	// Doodad scripting engine supervisor.
 	// NOTE: initialized and managed by the play_scene.