diff --git a/include/akgl/actor.h b/include/akgl/actor.h index 105050f..77ddd8b 100644 --- a/include/akgl/actor.h +++ b/include/akgl/actor.h @@ -124,6 +124,7 @@ typedef struct akgl_Actor { SDL_Time movetimer; /**< Unused. Nothing in the library reads or writes it. */ akgl_CollisionShape shape; /**< The collision volume this actor occupies. Copied from the character by akgl_actor_set_character unless #shape_override is set. Zeroed by akgl_actor_initialize, so an actor nobody configured does not collide. */ bool shape_override; /**< The game set #shape itself and akgl_actor_set_character must leave it alone. Distinct from "the shape is empty": an actor deliberately given no collider and an actor not yet given one are different states, and only this flag tells them apart. */ + struct akgl_CollisionProxy *proxy; /**< This actor's registration in the broad phase, or `NULL` when it has none. Owned by the library and released with the actor; a game neither sets nor frees it. */ // Velocity. Combined effect of all forces acting on the actor resulting // in energy along an axis. float32_t vx; /**< Velocity along x, world units per second. Recomputed each step as `ex + tx`; writing it directly is overwritten. */ diff --git a/include/akgl/collision.h b/include/akgl/collision.h index fe14059..28361fb 100644 --- a/include/akgl/collision.h +++ b/include/akgl/collision.h @@ -125,6 +125,73 @@ typedef struct akgl_CollisionShape { float32_t hz; /**< Half-extent along z. Never 0 after a setter has run; see the extrusion note on this file. */ } akgl_CollisionShape; +/** + * @brief One shape's registration in the broad phase. + * + * A proxy is what the partitioner indexes. It carries a **copy** of the shape + * rather than a pointer to it, which is deliberate: an actor's own logic may + * legitimately rewrite its shape mid-step -- a character crouching, a projectile + * arming -- and a broad phase whose bounds were computed from a shape that has + * since changed is a source of bugs that only appear when two things happen in + * the same frame. The copy is refreshed once per step, from one place. + * + * Proxies are pool objects. The library creates and destroys them; a game does + * not, and a game that releases an actor gets its proxy released with it. + */ +typedef struct akgl_CollisionProxy { + uint8_t refcount; /**< Pool bookkeeping; 0 means the slot is free. First member, as in every pooled type here. */ + struct akgl_Actor *owner; /**< The actor this proxy tracks, or `NULL` for static geometry with no actor behind it. Borrowed; no reference is taken. */ + akgl_CollisionShape shape; /**< A copy of the owner's shape as of the last refresh. */ + SDL_FRect bounds; /**< Planar world bounds in map pixels, computed from #shape at (#x, #y). */ + float32_t x; /**< World position #bounds was computed from. */ + float32_t y; /**< World position #bounds was computed from. */ + float32_t z; /**< World position #bounds was computed from. Not represented in #bounds. */ + uint32_t stamp; /**< Sweep serial this proxy was last visited on, so a proxy spanning several cells is reported once. Written by the partitioner. */ +} akgl_CollisionProxy; + +/* + * The following is part of the internal API. Proxies are created and destroyed + * by the library, not by a game. + */ + +/** + * @brief Claim a pooled proxy for an actor's shape and take the reference on it. + * + * Follows the convention every pool here uses: akgl_heap_next_collision_proxy + * finds a free slot and does **not** claim it, and this takes the reference. + * Write the two adjacent, because until the reference is taken the slot is still + * free and the next acquire hands out the same pointer. That is `TODO.md` "Known + * and still open" item 8, it applies to four of the five existing pools, and + * this one does not depart from it -- a sixth convention would be worse than the + * defect. + * + * @param obj The proxy to initialize. Required. + * @param owner The actor it tracks, or `NULL` for static geometry. + * @param shape The shape to copy into it. Required. + * @param x World position of the owner. + * @param y World position of the owner. + * @param z World position of the owner. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER If @p obj or @p shape is `NULL`. + */ +akerr_ErrorContext AKERR_NOIGNORE *akgl_collision_proxy_initialize(akgl_CollisionProxy *obj, struct akgl_Actor *owner, akgl_CollisionShape *shape, float32_t x, float32_t y, float32_t z); + +/** + * @brief Recompute a proxy's bounds from its owner's current shape and position. + * + * Called once per step per live proxy, before the broad phase is asked anything. + * This is the one place the shape copy is refreshed. + * + * @param obj The proxy. Required. + * @param shape The current shape to copy. Required. + * @param x Current world position of the owner. + * @param y Current world position of the owner. + * @param z Current world position of the owner. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER If @p obj or @p shape is `NULL`. + */ +akerr_ErrorContext AKERR_NOIGNORE *akgl_collision_proxy_sync(akgl_CollisionProxy *obj, akgl_CollisionShape *shape, float32_t x, float32_t y, float32_t z); + /** * @brief Build an axis-aligned box from the frame-relative rectangle a game has. * diff --git a/include/akgl/heap.h b/include/akgl/heap.h index fd92a42..598eb5e 100644 --- a/include/akgl/heap.h +++ b/include/akgl/heap.h @@ -29,6 +29,7 @@ #ifndef _AKGL_HEAP_H_ #define _AKGL_HEAP_H_ +#include #include #include #include @@ -50,6 +51,18 @@ #ifndef AKGL_MAX_HEAP_STRING #define AKGL_MAX_HEAP_STRING 256 #endif +/** + * @brief The collision proxy pool. + * + * Two per actor. One is the actor's own; the spare headroom is for static + * geometry a game registers that is not tile-aligned -- a slope, a Tiled object + * rectangle, a moving platform. Solid *tiles* need none of these: the broad + * phase reads them out of the tilemap's own array, because one proxy per tile + * would be tens of megabytes on a large map to index data that is already a grid. + */ +#ifndef AKGL_MAX_HEAP_COLLISION_PROXY +#define AKGL_MAX_HEAP_COLLISION_PROXY (AKGL_MAX_HEAP_ACTOR * 2) +#endif /** @brief The actor pool. Public so the render and physics sweeps can walk it directly instead of going through the registry. */ extern akgl_Actor akgl_heap_actors[AKGL_MAX_HEAP_ACTOR]; @@ -61,6 +74,8 @@ extern akgl_SpriteSheet akgl_heap_spritesheets[AKGL_MAX_HEAP_SPRITESHEET]; extern akgl_Character akgl_heap_characters[AKGL_MAX_HEAP_CHARACTER]; /** @brief The string pool. Every entry is PATH_MAX bytes, so this is the largest of the five by a wide margin. */ extern akgl_String akgl_heap_strings[AKGL_MAX_HEAP_STRING]; +/** @brief The collision proxy pool. Only the library allocates from it. */ +extern akgl_CollisionProxy akgl_heap_collision_proxies[AKGL_MAX_HEAP_COLLISION_PROXY]; /** * @brief Zero every pool, marking every slot free. @@ -218,4 +233,31 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_heap_release_character(akgl_Character *p */ akerr_ErrorContext AKERR_NOIGNORE *akgl_heap_release_string(akgl_String *ptr); +/** + * @brief Find a free collision proxy slot. + * + * Does **not** take the reference, matching four of the five pools above. + * akgl_collision_proxy_initialize takes it; write the two adjacent, because + * until it runs the slot still reads as free and the next acquire hands out the + * same pointer. + * + * @param dest Receives the proxy. Required. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKGL_ERR_HEAP If every slot is in use. That normally means proxies are + * not being released rather than that the pool is too small. + */ +akerr_ErrorContext AKERR_NOIGNORE *akgl_heap_next_collision_proxy(akgl_CollisionProxy **dest); + +/** + * @brief Give a collision proxy back. + * + * Decrements, and at zero clears the slot. A proxy holds no external resource + * and appears in no registry, so there is nothing else to unwind. + * + * @param ptr The proxy. Required. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER If @p ptr is `NULL`. + */ +akerr_ErrorContext AKERR_NOIGNORE *akgl_heap_release_collision_proxy(akgl_CollisionProxy *ptr); + #endif //_AKGL_HEAP_H_ diff --git a/src/collision_shape.c b/src/collision_shape.c index 3517294..158054c 100644 --- a/src/collision_shape.c +++ b/src/collision_shape.c @@ -181,3 +181,39 @@ akerr_ErrorContext *akgl_collision_shape_interacts(akgl_CollisionShape *self, ak *dest = ((other->layermask & self->collidemask) != 0); SUCCEED_RETURN(errctx); } + +akerr_ErrorContext *akgl_collision_proxy_initialize(akgl_CollisionProxy *obj, struct akgl_Actor *owner, akgl_CollisionShape *shape, float32_t x, float32_t y, float32_t z) +{ + PREPARE_ERROR(errctx); + FAIL_ZERO_RETURN(errctx, obj, AKERR_NULLPOINTER, "NULL proxy reference"); + FAIL_ZERO_RETURN(errctx, shape, AKERR_NULLPOINTER, "NULL shape reference"); + + memset(obj, 0x00, sizeof(akgl_CollisionProxy)); + obj->owner = owner; + + /* + * The reference is taken here and not in akgl_heap_next_collision_proxy, + * matching four of the five pools that came before. Until this runs the slot + * still reads as free -- which is also the property that keeps an acquire + * abandoned before this point from leaking the slot. + */ + obj->refcount = 1; + + PASS(errctx, akgl_collision_proxy_sync(obj, shape, x, y, z)); + SUCCEED_RETURN(errctx); +} + +akerr_ErrorContext *akgl_collision_proxy_sync(akgl_CollisionProxy *obj, akgl_CollisionShape *shape, float32_t x, float32_t y, float32_t z) +{ + PREPARE_ERROR(errctx); + FAIL_ZERO_RETURN(errctx, obj, AKERR_NULLPOINTER, "NULL proxy reference"); + FAIL_ZERO_RETURN(errctx, shape, AKERR_NULLPOINTER, "NULL shape reference"); + + // A copy, not a pointer. See the note on akgl_CollisionProxy. + obj->shape = *shape; + obj->x = x; + obj->y = y; + obj->z = z; + PASS(errctx, akgl_collision_shape_bounds(&obj->shape, x, y, &obj->bounds)); + SUCCEED_RETURN(errctx); +} diff --git a/src/heap.c b/src/heap.c index 758d7bb..4c6c1b3 100644 --- a/src/heap.c +++ b/src/heap.c @@ -19,6 +19,7 @@ akgl_Sprite akgl_heap_sprites[AKGL_MAX_HEAP_SPRITE]; akgl_SpriteSheet akgl_heap_spritesheets[AKGL_MAX_HEAP_SPRITESHEET]; akgl_Character akgl_heap_characters[AKGL_MAX_HEAP_CHARACTER]; akgl_String akgl_heap_strings[AKGL_MAX_HEAP_STRING]; +akgl_CollisionProxy akgl_heap_collision_proxies[AKGL_MAX_HEAP_COLLISION_PROXY]; akerr_ErrorContext *akgl_heap_init(void) { @@ -37,6 +38,9 @@ akerr_ErrorContext *akgl_heap_init(void) for ( i = 0; i < AKGL_MAX_HEAP_STRING; i++) { memset(&akgl_heap_strings[i], 0x00, sizeof(akgl_String)); } + for ( i = 0; i < AKGL_MAX_HEAP_COLLISION_PROXY; i++) { + memset(&akgl_heap_collision_proxies[i], 0x00, sizeof(akgl_CollisionProxy)); + } SUCCEED_RETURN(errctx); } @@ -129,6 +133,16 @@ akerr_ErrorContext *akgl_heap_release_actor(akgl_Actor *ptr) PASS(errctx, akgl_heap_release_actor(ptr->children[i])); } } + /* + * The proxy goes with the actor. It holds a borrowed `owner` pointer + * into the slot about to be zeroed, so leaving it registered would give + * the broad phase a proxy whose owner reads as a free actor -- and the + * next contact against it would report a collision with nothing. + */ + if ( ptr->proxy != NULL ) { + PASS(errctx, akgl_heap_release_collision_proxy(ptr->proxy)); + ptr->proxy = NULL; + } SDL_ClearProperty(AKGL_REGISTRY_ACTOR, (char *)&ptr->name); memset(ptr, 0x00, sizeof(akgl_Actor)); } @@ -212,3 +226,30 @@ akerr_ErrorContext *akgl_heap_release_string(akgl_String *ptr) } SUCCEED_RETURN(errctx); } + +akerr_ErrorContext *akgl_heap_next_collision_proxy(akgl_CollisionProxy **dest) +{ + PREPARE_ERROR(errctx); + FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL destination reference"); + for (int i = 0; i < AKGL_MAX_HEAP_COLLISION_PROXY; i++ ) { + if ( akgl_heap_collision_proxies[i].refcount != 0 ) { + continue; + } + *dest = &akgl_heap_collision_proxies[i]; + SUCCEED_RETURN(errctx); + } + FAIL_RETURN(errctx, AKGL_ERR_HEAP, "Unable to find unused collision proxy on the heap"); +} + +akerr_ErrorContext *akgl_heap_release_collision_proxy(akgl_CollisionProxy *ptr) +{ + PREPARE_ERROR(errctx); + FAIL_ZERO_RETURN(errctx, ptr, AKERR_NULLPOINTER, "NULL collision proxy reference"); + if ( ptr->refcount > 0 ) { + ptr->refcount -= 1; + } + if ( ptr->refcount == 0 ) { + memset(ptr, 0x00, sizeof(akgl_CollisionProxy)); + } + SUCCEED_RETURN(errctx); +} diff --git a/tests/collision.c b/tests/collision.c index 594f535..9e8a793 100644 --- a/tests/collision.c +++ b/tests/collision.c @@ -15,8 +15,11 @@ #include +#include #include #include +#include +#include #include "testutil.h" @@ -305,6 +308,145 @@ akerr_ErrorContext *test_shape_interacts(void) SUCCEED_RETURN(errctx); } +/** + * @brief The pool convention, and the window it deliberately leaves open. + */ +akerr_ErrorContext *test_proxy_pool(void) +{ + akgl_CollisionProxy *first = NULL; + akgl_CollisionProxy *second = NULL; + akgl_CollisionShape shape; + SDL_FRect body = { .x = 0.0f, .y = 0.0f, .w = 16.0f, .h = 16.0f }; + akerr_ErrorContext *inner = NULL; + int claimed = 0; + int i = 0; + + PREPARE_ERROR(errctx); + + ATTEMPT { + CATCH(errctx, akgl_heap_init()); + CATCH(errctx, akgl_collision_shape_box(&shape, &body, 0.0f)); + + /* + * Acquiring does not claim the slot, and that is the convention rather + * than an oversight: an acquire abandoned before initialization leaks + * nothing, because the slot still reads as free. The cost is this window, + * where two acquires in a row return the same pointer -- which is why the + * library writes the acquire and the initialize adjacent. + */ + CATCH(errctx, akgl_heap_next_collision_proxy(&first)); + TEST_ASSERT(errctx, (first->refcount == 0), + "acquiring a proxy claimed the slot; that is not this pool's convention"); + CATCH(errctx, akgl_heap_next_collision_proxy(&second)); + TEST_ASSERT(errctx, (first == second), + "two acquires with no initialize between them returned different slots"); + + // Initializing is what closes it. + CATCH(errctx, akgl_collision_proxy_initialize(first, NULL, &shape, 10.0f, 20.0f, 0.0f)); + TEST_ASSERT(errctx, (first->refcount == 1), "initializing did not take the reference"); + CATCH(errctx, akgl_heap_next_collision_proxy(&second)); + TEST_ASSERT(errctx, (first != second), "an initialized slot was handed out again"); + + // The proxy carries a copy of the shape, not a pointer to it, so a later + // edit to the caller's shape cannot change what the broad phase indexed. + shape.hx = 999.0f; + TEST_ASSERT_FEQ(errctx, first->shape.hx, 8.0f, + "the proxy tracked an edit to the caller's shape; it holds %f", first->shape.hx); + // Body {0,0,16,16} centres at (8,8) with half-extents 8, so at owner x=10 + // the bounds start at 10 and are 16 wide. + TEST_ASSERT_FEQ(errctx, first->bounds.x, 10.0f, + "proxy bounds x is %f, expected 10", first->bounds.x); + TEST_ASSERT_FEQ(errctx, first->bounds.y, 20.0f, + "proxy bounds y is %f, expected 20", first->bounds.y); + TEST_ASSERT_FEQ(errctx, first->bounds.w, 16.0f, + "proxy bounds w is %f, expected 16", first->bounds.w); + + CATCH(errctx, akgl_heap_release_collision_proxy(first)); + TEST_ASSERT(errctx, (first->refcount == 0), "releasing did not free the slot"); + + // Exhaustion is an ordinary reported error, not a crash and not a silent + // hand-out of a slot somebody else is using. + CATCH(errctx, akgl_heap_init()); + for ( i = 0; i < (AKGL_MAX_HEAP_COLLISION_PROXY + 4); i++ ) { + inner = akgl_heap_next_collision_proxy(&first); + if ( inner != NULL ) { + break; + } + inner = akgl_collision_proxy_initialize(first, NULL, &shape, 0.0f, 0.0f, 0.0f); + if ( inner != NULL ) { + break; + } + claimed += 1; + } + TEST_ASSERT(errctx, (inner != NULL), "the proxy pool never ran out"); + TEST_EXPECT_STATUS(errctx, AKGL_ERR_HEAP, inner, "exhausting the proxy pool"); + inner = NULL; + TEST_ASSERT(errctx, (claimed == AKGL_MAX_HEAP_COLLISION_PROXY), + "the pool handed out %d of %d slots before refusing", + claimed, AKGL_MAX_HEAP_COLLISION_PROXY); + + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, akgl_heap_next_collision_proxy(NULL), + "acquiring into no destination"); + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, akgl_heap_release_collision_proxy(NULL), + "releasing no proxy"); + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_collision_proxy_initialize(NULL, NULL, &shape, 0.0f, 0.0f, 0.0f), + "initializing no proxy"); + } CLEANUP { + if ( inner != NULL ) { + inner->handled = true; + inner = akerr_release_error(inner); + } + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + +/** + * @brief A proxy must not outlive the actor it points at. + * + * akgl_heap_release_actor zeroes the actor slot. A proxy left registered would + * hold a borrowed `owner` into a slot that now reads as free, and the next + * contact against it would be a collision with nothing -- so the release has to + * reach through and take the proxy with it. + */ +akerr_ErrorContext *test_proxy_dies_with_its_actor(void) +{ + akgl_Actor *actor = NULL; + akgl_CollisionProxy *proxy = NULL; + akgl_CollisionShape shape; + SDL_FRect body = { .x = 0.0f, .y = 0.0f, .w = 16.0f, .h = 16.0f }; + + PREPARE_ERROR(errctx); + + ATTEMPT { + CATCH(errctx, akgl_heap_init()); + CATCH(errctx, akgl_registry_init_actor()); + CATCH(errctx, akgl_collision_shape_box(&shape, &body, 0.0f)); + + CATCH(errctx, akgl_heap_next_actor(&actor)); + CATCH(errctx, akgl_actor_initialize(actor, "collider")); + CATCH(errctx, akgl_heap_next_collision_proxy(&proxy)); + CATCH(errctx, akgl_collision_proxy_initialize(proxy, actor, &shape, 0.0f, 0.0f, 0.0f)); + actor->proxy = proxy; + + TEST_ASSERT(errctx, (proxy->refcount == 1), "the proxy was not claimed"); + CATCH(errctx, akgl_heap_release_actor(actor)); + TEST_ASSERT(errctx, (proxy->refcount == 0), + "releasing the actor left its proxy registered, pointing at a freed slot"); + TEST_ASSERT(errctx, (proxy->owner == NULL), "the released proxy still names an owner"); + + // An actor with no proxy releases without complaint, which is every + // actor a game written before this existed. + CATCH(errctx, akgl_heap_next_actor(&actor)); + CATCH(errctx, akgl_actor_initialize(actor, "plain")); + CATCH(errctx, akgl_heap_release_actor(actor)); + } CLEANUP { + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + int main(void) { PREPARE_ERROR(errctx); @@ -316,6 +458,8 @@ int main(void) CATCH(errctx, test_shape_extrusion_beats_planar_penetration()); CATCH(errctx, test_shape_bounds()); CATCH(errctx, test_shape_interacts()); + CATCH(errctx, test_proxy_pool()); + CATCH(errctx, test_proxy_dies_with_its_actor()); } CLEANUP { } PROCESS(errctx) { } FINISH_NORETURN(errctx);