Refactor to remove dependency of DrivenEngine on World

Docs/Module-Dependencies-in-Luprex.md

@@ -0,0 +1,62 @@
+# Module Dependencies in Luprex
+
+Modules are listed in dependency order — each module's dependencies
+all appear earlier in the list. Where a dependency comes only from
+the `.cpp` file (not the `.hpp`), it is marked **(cpp-only)**.
+
+- **bytell-hash-map** — third-party hash map (header-only)
+- **eng-malloc** — custom deterministic memory allocator
+- **enginewrapper** — pure C interface for driver/driven boundary
+- **fast-float** — third-party float parser (header-only)
+- **flat-hash-map** — third-party hash map (header-only)
+- **spookyv2** — hash function
+- **util** → spookyv2
+- **luastack** → util
+- **luavector** → luastack
+- **traceback** → luastack
+- **debugcollector** → util
+- **streambuffer** → eng-malloc, luastack, util
+- **table** → luastack
+- **drivenengine** → enginewrapper, invocation, streambuffer, util
+- **json** → luastack, util
+- **http** → drivenengine, json(cpp-only), luastack, streambuffer
+- **planemap** → luastack, util
+- **luasnap** → luastack, streambuffer
+- **serializelua** → luastack, streambuffer
+- **sched** → luastack, streambuffer
+- **idalloc** → debugcollector, luastack, streambuffer
+- **invocation** → enginewrapper, streambuffer
+- **pprint** → luastack, table, util
+- **source** → debugcollector, luastack, luasnap, streambuffer, table(cpp-only), traceback, util
+- **animqueue** → debugcollector, luastack, streambuffer, util
+- **printbuffer** → debugcollector, invocation, streambuffer, util
+- **world** → animqueue, debugcollector, http, idalloc, invocation, luasnap, luastack, planemap, printbuffer, pprint, sched, serializelua, source, streambuffer, table, traceback
+- **lpxclient** → drivenengine, invocation, printbuffer, util, world
+- **lpxserver** → drivenengine, luastack, printbuffer, util, world
+- **eng-tests** → drivenengine, streambuffer, world
+
+## Observations
+
+### http depends on drivenengine (header-level)
+
+`http.hpp` includes `drivenengine.hpp` because it uses
+`SharedChannel` (defined in drivenengine). Meanwhile, `world`
+depends on both `http` and `drivenengine`, and `drivenengine`
+depends on `world`. This creates a layering tangle:
+
+```
+world -> http -> drivenengine -> world
+```
+
+The `SharedChannel` type (`std::shared_ptr<Channel>`) is really an
+I/O concept from the driver boundary. Moving `SharedChannel` and
+`Channel` into a smaller, lower-level header (perhaps
+`enginewrapper.hpp` or a new `channel.hpp`) would let `http` drop
+its dependency on `drivenengine` entirely.
+
+### world is a mega-consumer (16 dependencies)
+
+`world` depends on nearly every other module. This is expected for
+the central game-state container, but it does make `world` hard to
+test in isolation. The `world-*.cpp` split into multiple files
+helps readability but doesn't reduce coupling.

luprex/cpp/core/drivenengine.cpp

@@ -2,7 +2,7 @@
 #include "wrap-vector.hpp"
 #include "util.hpp"
 #include "drivenengine.hpp"
-#include "world.hpp"
+#include "animqueue.hpp"
 #include "base-buffer.hpp"
 
 #include <string_view>
@@ -56,10 +56,10 @@ Channel *DrivenEngine::get_chid(int chid) const {
     return channels_[chid].get();
 }
 
-static DrivenEngine *make_engine(std::string_view kind) {
+static DrivenEngine *make_engine(std::string_view kind, EngineWrapper *w) {
     for (auto reg = DrivenEngineReg::All; reg != nullptr; reg=reg->next) {
         if (kind == reg->name) {
-            UniqueDrivenEngine result = reg->maker();
+            UniqueDrivenEngine result = reg->maker(w);
             return result.release();
         }
     }
@@ -386,43 +386,6 @@ int64_t DrivenEngine::drv_get_actor_id() const {
     return actor_id_;
 }
 
-void DrivenEngine::drv_get_tangibles_near(int64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids) {
-    uint32_t hash1 = eng::memhash();
-    scan_result_.clear();
-    if (world_ && (tanid != 0)) {
-        PlaneScan scan;
-        scan.set_near(tanid, true);
-        scan.set_omit_nowhere(true);
-        scan.set_sorted(false);
-        scan.set_radius(util::XYZ(rx, ry, rz));
-        scan.set_shape(PlaneScan::CYLINDER);
-        world_->get_near(scan, &scan_result_);
-    }
-    *count = scan_result_.size();
-    if (*count > 0) {
-        *ids = &scan_result_[0];
-    } else {
-        *ids = nullptr;
-    }
-    uint32_t hash2 = eng::memhash();
-    assert(hash1 == hash2);
-}
-
-void DrivenEngine::drv_get_animation_queues(uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings) {
-    anim_queues_.resize(count);
-
-    if (!world_) {
-        for (int i = 0; i < int(count); i++) {
-            anim_queues_[i] = AnimQueue::get_encoded_blank_queue();
-        }
-    } else {
-        world_->get_encoded_animation_queues(count, ids, anim_queues_);
-    }
-    for (int i = 0; i < int(count); i++) {
-        lengths[i] = anim_queues_[i]->size();
-        strings[i] = anim_queues_[i]->c_str();
-    }
-}
 
 //////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////
@@ -541,13 +504,6 @@ static uint64_t drv_get_actor_id(EngineWrapper *w) {
     return w->engine->drv_get_actor_id();
 }
 
-static void drv_get_tangibles_near(EngineWrapper *w, uint64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids) {
-    return w->engine->drv_get_tangibles_near(tanid, rx, ry, rz, count, ids);
-}
-
-static void drv_get_animation_queues(EngineWrapper *w, uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings) {
-    return w->engine->drv_get_animation_queues(count, ids, lengths, strings);
-}
 
 
 //////////////////////////////////////////////////////////////////////////////
@@ -588,7 +544,7 @@ static void play_initialize(EngineWrapper *w, const char *engtype, const char *l
     }
 
     // Create the engine of the appropriate type.
-    w->engine = make_engine(engtype);
+    w->engine = make_engine(engtype, w);
     if (w->engine == nullptr) {
         return reset_wrapper(w, "No such driven engine type: %s", engtype);
     }
@@ -604,7 +560,7 @@ static void replay_initialize(EngineWrapper *w) {
     }
 
     // Create the engine.
-    w->engine = make_engine(engtype.c_str());
+    w->engine = make_engine(engtype.c_str(), w);
     if (w->engine == nullptr) {
         return reset_wrapper(w, "No such driven engine type: %s", engtype.c_str());
     }
@@ -888,6 +844,27 @@ static void replaycore_step(EngineWrapper *w) {
 //////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////
 
+static int64_t empty_id_list_ = 0;
+
+static void default_get_tangibles_near(EngineWrapper *, uint64_t, double, double, double, uint32_t *count, int64_t **ids) {
+    *count = 0;
+    *ids = &empty_id_list_;
+}
+
+static void default_get_animation_queues(EngineWrapper *, uint32_t count, const int64_t *, uint32_t *lengths, const char **strings) {
+    util::SharedStdString blank = AnimQueue::get_encoded_blank_queue();
+    for (int i = 0; i < int(count); i++) {
+        lengths[i] = blank->size();
+        strings[i] = blank->c_str();
+    }
+}
+
+void DrivenEngine::unexpose_world_to_driver(EngineWrapper *w) {
+    w->world = nullptr;
+    w->get_tangibles_near = default_get_tangibles_near;
+    w->get_animation_queues = default_get_animation_queues;
+}
+
 static void init_engine_wrapper_helper(EngineWrapper *w) {
     static bool called_initializer;
     assert(DrivenEngineInitializerReg::func != nullptr);
@@ -909,8 +886,8 @@ static void init_engine_wrapper_helper(EngineWrapper *w) {
     w->get_rescan_lua_source = drv_get_rescan_lua_source;
     w->get_stop_driver = drv_get_stop_driver;
     w->get_actor_id = drv_get_actor_id;
-    w->get_tangibles_near = drv_get_tangibles_near;
+    w->get_tangibles_near = default_get_tangibles_near;
-    w->get_animation_queues = drv_get_animation_queues;
+    w->get_animation_queues = default_get_animation_queues;
 
     w->play_initialize = play_initialize;
     w->play_clear_new_outgoing = play_clear_new_outgoing;

luprex/cpp/core/drivenengine.hpp

@@ -52,15 +52,14 @@
 #include "util.hpp"
 #include "streambuffer.hpp"
 #include "enginewrapper.hpp"
-#include "planemap.hpp"
 #include "invocation.hpp"
 
 class DrivenEngine;
-class World;
 using UniqueDrivenEngine = std::unique_ptr<DrivenEngine>;
-using DrivenEngineMaker = UniqueDrivenEngine (*)();
+using DrivenEngineMaker = UniqueDrivenEngine (*)(EngineWrapper *);
 using DrivenEngineInitializer = void (*)();
 
+
 class Channel : public eng::opnew {
 public:
     // Get the buffers associated with this channel.
@@ -222,6 +221,12 @@ public:
     //
     //////////////////////////////////////////////////////////////
 
+    // Reset the wrapper's world-related function pointers back to
+    // defaults that return empty results.  Call this before destroying
+    // a World that was previously exposed via World::setup_wrapper.
+    //
+    static void unexpose_world_to_driver(EngineWrapper *w);
+
     // Constructor.
     //
     // Most initialization is achieved by 'drv_xxx' functions, so
@@ -260,8 +265,6 @@ public:
     bool drv_get_rescan_lua_source() const;
     bool drv_get_stop_driver() const;
     int64_t drv_get_actor_id() const;
-    void drv_get_tangibles_near(int64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids);
-    void drv_get_animation_queues(uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings);
 
     void drv_clear_new_outgoing();
     void drv_sent_outgoing(uint32_t chid, uint32_t nbytes);
@@ -280,15 +283,6 @@ private:
     Channel *get_chid(int chid) const;
     
 protected:
-    // The DrivenEngine can optionally contain
-    // a world model.  This is initialized to
-    // nullptr, but classes that derive from
-    // DrivenEngine can store a world model here.
-    // If they do, then functions like get_near
-    // will reference this model.
-    // 
-    std::unique_ptr<World> world_;
-
     // When the Driver calls get_actor_id,
     // we return this value.  This is initialized
     // to zero, but classes that derive from
@@ -302,8 +296,6 @@ private:
     eng::vector<SharedChannel> accepted_channels_;
     eng::vector<uint32_t> new_outgoing_;
     eng::vector<uint32_t> listen_ports_;
-    util::IdVector scan_result_;
-    std::vector<util::SharedStdString> anim_queues_;
     StreamBuffer call_function_retpk_;
     bool rescan_lua_source_ = false;
     double clock_ = 0.0;
@@ -324,8 +316,8 @@ struct DrivenEngineReg {
 };
 
 #define DrivenEngineDefine(name, cname) \
-    UniqueDrivenEngine dengmake_##cname() { \
+    UniqueDrivenEngine dengmake_##cname(EngineWrapper *w) { \
-        return UniqueDrivenEngine(new cname); \
+        return UniqueDrivenEngine(new cname(w)); \
     } \
     DrivenEngineReg dengreg_##cname(name, dengmake_##cname);
 

luprex/cpp/core/eng-tests.cpp

@@ -23,7 +23,7 @@ static void dump_lines(StreamBuffer *in, int chid) {
 class DriverWebServerTest : public DrivenEngine {
 public:
     eng::vector<SharedChannel> channels_;
-    DriverWebServerTest() {
+    DriverWebServerTest(EngineWrapper *) {
         SharedChannel ch = new_outgoing_channel("cert:stanford.edu:443");
         ch->out()->write_bytes("GET https://stanford.edu/xbanankjdsh.html HTTP/1.1\n\n");
         channels_.emplace_back(std::move(ch));
@@ -47,7 +47,7 @@ public:
 class DriverDNSFailTest : public DrivenEngine {
 public:
     eng::vector<SharedChannel> channels_;
-    DriverDNSFailTest() {
+    DriverDNSFailTest(EngineWrapper *) {
         SharedChannel ch = new_outgoing_channel("akjsdkajshdakjshd.alk:80");
         ch->out()->write_bytes("GET http://stanford.edu/index.html HTTP/1.1\n\n");
         channels_.emplace_back(std::move(ch));
@@ -72,7 +72,7 @@ class DriverPrintClockTest : public DrivenEngine {
 public:
     int count_;
     double last_clock_;
-    DriverPrintClockTest() {
+    DriverPrintClockTest(EngineWrapper *) {
         count_ = 0;
         last_clock_ = 0.0;
     }
@@ -93,7 +93,7 @@ class RunUnitTests : public DrivenEngine {
 public:
     UniqueWorld world_;
 
-    RunUnitTests() {
+    RunUnitTests(EngineWrapper *) {
         world_.reset(new World(WORLD_TYPE_MASTER));
         rescan_lua_source(true);
     }

luprex/cpp/core/enginewrapper.hpp

@@ -40,11 +40,13 @@ enum class AccessKind {
 class DrivenEngine;
 class PlayLogfile;
 class ReplayLogfile;
+class World;
 
 struct EngineWrapper {
     char error[DRV_ERRMSG_SIZE];
     char databuffer[DRV_SHORTSTRING_SIZE];
     DrivenEngine *engine;
+    World *world;
     PlayLogfile *wlog;
     ReplayLogfile *rlog;
 

luprex/cpp/core/lpxclient.cpp

@@ -11,6 +11,8 @@
 
 class LpxClient : public DrivenEngine {
 public:
+    EngineWrapper *wrapper_;
+    std::unique_ptr<World> world_;
     InvocationQueue unack_;
     SharedChannel channel_;
     PrintChanneler print_channeler_;
@@ -18,19 +20,22 @@ public:
     lua_State *lua_syntax_checker_;
 
 public:
-    LpxClient() {
+    LpxClient(EngineWrapper *w) : wrapper_(w) {
         lua_syntax_checker_ = LuaCoreStack::newstate(eng::l_alloc);
 
         set_initial_state_standalone();
     }
 
     ~LpxClient() {
+        unexpose_world_to_driver(wrapper_);
         lua_close(lua_syntax_checker_);
     }
 
     void set_initial_state_connect(const eng::string &hostspec) {
         // Create the world model.
+        unexpose_world_to_driver(wrapper_);
         world_.reset(new World(WORLD_TYPE_PREDICTIVE));
+        world_->expose_world_to_driver(wrapper_);
 
         // Create the communication channel.
         channel_ = new_outgoing_channel(hostspec);
@@ -57,7 +62,9 @@ public:
 
     void set_initial_state_standalone() {
         // Create the world model.
+        unexpose_world_to_driver(wrapper_);
         world_.reset(new World(WORLD_TYPE_MASTER));
+        world_->expose_world_to_driver(wrapper_);
 
         // Make sure the channel is empty.
         channel_.reset();

luprex/cpp/core/lpxserver.cpp

@@ -23,6 +23,8 @@ using ClientVector = eng::vector<UniqueClient>;
 
 class LpxServer : public DrivenEngine {
 public:
+    EngineWrapper *wrapper_;
+    std::unique_ptr<World> world_;
     ClientVector clients_;
     PrintChanneler print_channeler_;
     HttpChannelMap http_client_channels_;
@@ -32,13 +34,14 @@ public:
     lua_State *lua_syntax_checker_;
 
 public:
-    LpxServer()
+    LpxServer(EngineWrapper *w) : wrapper_(w)
     {
         // Create a little lua interpreter for syntax checking only.
         lua_syntax_checker_ = LuaCoreStack::newstate(eng::l_alloc);
 
         // Create the master world model.
         world_.reset(new World(WORLD_TYPE_MASTER));
+        world_->expose_world_to_driver(wrapper_);
 
         // Create the admin actor.  Note: there isn't any 'init' function yet.
         actor_id_ = world_->create_login_actor();
@@ -60,6 +63,7 @@ public:
     }
 
     ~LpxServer() {
+        unexpose_world_to_driver(wrapper_);
         lua_close(lua_syntax_checker_);
     }
 

luprex/cpp/core/table.cpp

@@ -2,7 +2,6 @@
 #include "wrap-string.hpp"
 
 #include "table.hpp"
-#include "source.hpp"
 
 // A quick check to see if a table appears to be a array.
 // Does not thoroughly verify the array.  Returns the size

luprex/cpp/core/world-core.cpp

@@ -1,5 +1,6 @@
 
 #include "world.hpp"
+#include "enginewrapper.hpp"
 #include "idalloc.hpp"
 #include "animqueue.hpp"
 #include "traceback.hpp"
@@ -301,18 +302,6 @@ void World::get_near(int64_t player_id, float radius, bool exclude_nowhere, bool
     get_near(scan, into);
 }
 
-void World::get_encoded_animation_queues(uint32_t count, const int64_t *ids, util::SharedStdStringVec &into) {
-    into.resize(count);
-    for (int i = 0; i < int(count); i++) {
-        Tangible *tan = tangible_get(ids[i]);
-        if (tan == nullptr) {
-            into[i] = AnimQueue::get_encoded_blank_queue();
-        } else {
-            into[i] = tan->anim_queue_.get_encoded_queue();
-        }
-    }
-}
-
 World::Redirects World::fetch_redirects() {
     World::Redirects result = std::move(redirects_);
     redirects_.clear();
@@ -1293,3 +1282,51 @@ void engine_initialization() {
 }
 
 static DrivenEngineInitializerReg eireg(engine_initialization);
+
+//////////////////////////////////////////////////////////////////////////////
+//
+// Functions that allow the Driver to Peer Directly into the World.
+//
+//////////////////////////////////////////////////////////////////////////////
+
+void World::get_animation_queues(uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings) {
+    wrapper_anim_queues_.resize(count);
+    for (int i = 0; i < int(count); i++) {
+        Tangible *tan = tangible_get(ids[i]);
+        if (tan == nullptr) {
+            wrapper_anim_queues_[i] = AnimQueue::get_encoded_blank_queue();
+        } else {
+            wrapper_anim_queues_[i] = tan->anim_queue_.get_encoded_queue();
+        }
+        lengths[i] = wrapper_anim_queues_[i]->size();
+        strings[i] = wrapper_anim_queues_[i]->c_str();
+    }
+}
+
+void World::get_tangibles_near(uint64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids) {
+    uint32_t hash1 = eng::memhash();
+    wrapper_scan_result_.clear();
+    if (tanid != 0) {
+        PlaneScan scan;
+        scan.set_near(tanid, true);
+        scan.set_omit_nowhere(true);
+        scan.set_sorted(false);
+        scan.set_radius(util::XYZ(rx, ry, rz));
+        scan.set_shape(PlaneScan::CYLINDER);
+        get_near(scan, &wrapper_scan_result_);
+    }
+    *count = wrapper_scan_result_.size();
+    *ids = &wrapper_scan_result_[0];
+    uint32_t hash2 = eng::memhash();
+    assert(hash1 == hash2);
+}
+
+void World::expose_world_to_driver(EngineWrapper *w) {
+    w->world = this;
+    w->get_tangibles_near = [](EngineWrapper *w, uint64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids) {
+        w->world->get_tangibles_near(tanid, rx, ry, rz, count, ids);
+    };
+    w->get_animation_queues = [](EngineWrapper *w, uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings) {
+        w->world->get_animation_queues(count, ids, lengths, strings);
+    };
+}

luprex/cpp/core/world.hpp

@@ -22,6 +22,8 @@
 #include "source.hpp"
 #include "luasnap.hpp"
 
+struct EngineWrapper;
+
 enum WorldType {
     WORLD_TYPE_MASTER = 1,
     WORLD_TYPE_PREDICTIVE = 2,
@@ -169,11 +171,21 @@ public:
     //
     void get_near(int64_t player_id, float radius, bool exclude_nowhere, bool omit_player, bool sorted, IdVector *into) const;
 
-    // get encoded animation queues.
+    // Get tangibles near the specified tangible.
     //
-    // This is used by the graphics engine to get the animation queues.
+    // Returns a count and pointer to an array of tangible IDs.  The
+    // returned pointer remains valid until the next call to
+    // get_tangibles_near.
     //
-    void get_encoded_animation_queues(uint32_t count, const int64_t *ids, util::SharedStdStringVec &into);
+    void get_tangibles_near(uint64_t tanid, double rx, double ry, double rz, uint32_t *count, int64_t **ids);
+
+    // Get the animation queues for the specified tangibles.
+    //
+    // For each tangible ID, returns the animation queue as a serialized
+    // string via lengths/strings output arrays.  The returned pointers
+    // remain valid until the next call to get_animation_queues.
+    //
+    void get_animation_queues(uint32_t count, const int64_t *ids, uint32_t *lengths, const char **strings);
 
     // Make a tangible.
     //
@@ -309,6 +321,10 @@ public:
     //
     void run_unittests();
     
+    // Install this world into an EngineWrapper's function pointers.
+    //
+    void expose_world_to_driver(EngineWrapper *w);
+
     // fetch_global_pointer
     //
     // Given a lua state, fetch the world model associated with
@@ -668,6 +684,12 @@ private:
     //
     Redirects redirects_;
 
+    // Storage for wrapper_get_tangibles_near and wrapper_get_animation_queues.
+    // These hold results alive while the driver reads from the raw pointers.
+    //
+    util::IdVector wrapper_scan_result_;
+    util::SharedStdStringVec wrapper_anim_queues_;
+
     // lthread variables: see set_lthread_state for explanation.
     //
     int64_t lthread_actor_id_;