Merge pull request #103 from chnsw/fix/host-pool-connect-port

Fix activity host connections and spawn ordering during travel

Sunrise/resources/default_settings.json

@@ -1,5 +1,5 @@
 {
-  "version": 15,
+  "version": 16,
   "core": {
     "logging": {
       "debugger_sink": true,

Sunrise/src/client/content/activity/activity_sdk_generation_worker.h

@@ -62,7 +62,7 @@ struct OfflineBuildResult final {
                                                void* progressContext,
                                                OfflineBuildResult& output) noexcept;
 
-/** Immutable boot policy for the live generator. Each output nothing reads is opt-in. */
+/** Immutable boot policy for the live generator. Its pack backs host roster mission seeds. */
 struct Policy final {
     bool enabled{};
     /** Writes the sdk/lua declaration tree, which no runtime loads. */

Sunrise/src/core/settings/settings.h

@@ -13,10 +13,10 @@
 
 namespace sunrise::core::settings {
 
-/** Boot policy for the optional full-estate activity SDK generator. */
+/** Boot policy for generating the activity SDK required by host roster construction. */
 struct ActivitySdkGenerationSettings final {
-    /** Allows generation work to be requested. Off unless the settings file opts in. */
-    bool enabled{false};
+    /** On by default like the bundled file, so a file without this block still generates. */
+    bool enabled{true};
     /** Writes the sdk/lua declaration tree. On by default, and only runs when generation does. */
     bool luaDeclarations{true};
 };
@@ -26,7 +26,7 @@ struct ActivitySdkGenerationSettings final {
  * Raise it when a key is renamed, removed, changes meaning, or must take a new default. Adding a
  * key needs no raise, because a missing key already takes its default.
  */
-inline constexpr std::uint32_t kSettingsVersion = 15;
+inline constexpr std::uint32_t kSettingsVersion = 16;
 
 /** Parsed read-only process settings. */
 struct Settings {

Sunrise/src/core/settings/settings_parser.cpp

@@ -111,7 +111,7 @@ bool Parser::core(Settings& output) noexcept {
     }
 }
 
-/** Parses the Core-owned, fail-closed activity SDK generation gate. */
+/** Parses the Core-owned activity SDK generation gate. Omitted members keep their defaults. */
 bool Parser::activity_sdk_generation_settings(ActivitySdkGenerationSettings& output) noexcept {
     if (!consume('{')) {
         return false;

Sunrise/src/core/settings/settings_upgrade.cpp

@@ -39,14 +39,16 @@ constexpr std::array<ReplacedMember, 11> kReplacedMembers{{
     // Version 8 turned the flat payout list into rows filtered by rarity, gear class and
     // masterwork state.
     {"\"dismantle_rewards\"", 8},
-    // Version 13 turned these three on. A file that never carried them takes the new default; one
+    // Version 13 turned these two on. A file that never carried them takes the new default; one
     // that carried the old value is corrected here.
-    {"\"lua_declarations\"", 13},
     {"\"suppress_peer_relay\"", 13},
     {"\"activity_public_membership\"", 13},
     // Version 15 seeded the lore book unlock slots, so both banks take the new default.
     {"\"character_flags\"", 15},
     {"\"objective_values\"", 15},
+    // Version 16 turned generation on. The whole block is replaced, because "enabled" is not
+    // unique in the document. The block also carries the lua_declarations default of version 13.
+    {"\"activity_sdk_generation\"", 16},
 }};
 
 /** One renamed member, and the layout version that renamed it. */

Sunrise/src/server/bap/encrypted/push/activity/activity_roster_push.cpp

@@ -291,8 +291,9 @@ bool append_roster_notification(
     const bool lifetimePending =
         hasScriptablePending && singleScriptableLink
         && scriptablePending.kind == server::activity::host::ScriptableOverrideKind::lifetime;
-    // The loading lifetime holds only until the destination region is instantiated. The stricter
-    // in-world state is reported after spawning and would make this field wait on its own result.
+    // The loading lifetime is the presentation, not the spawn hold: state 4 shows the loading
+    // screen and refuses the native spawn gate on its own, so it is released once the region is
+    // instantiated. `awaiting_client_sync` carries the hold on to the client's arrival report.
     // An explicit lifetime request still wins.
     const bool clientLoading = !client_region_ready(session, refresh);
     const bool bodyPending = hasScriptablePending && singleScriptableLink && !lifetimePending;

Sunrise/src/server/bap/encrypted/push/activity/activity_roster_snapshot.cpp

@@ -98,9 +98,10 @@ bool client_region_ready(const Session& session, const RefreshReport* refresh) n
     return !movePending && held >= 0;
 }
 
-/** Tests whether the client has completed spawning into its instantiated region. */
+/** Tests whether the client has reported arrival in its instantiated region. */
 bool client_in_world(const Session& session, const RefreshReport* refresh) noexcept {
-    // World-state 8 is post-spawn, so no roster field that releases the spawn may read it.
+    // WS-702 world-state 8 follows the bootflow's arrival, independently of the player spawn.
+    // Holding a region alone can precede that report and the world-transition fade's final arm.
     const state::activity::membership::ClientPlacement placement =
         client_placement(session, refresh);
     return placement.entered && client_region_ready(session, refresh);
@@ -607,9 +608,10 @@ build_roster_snapshot(Session& session,
     // carries matches nothing.
     snapshot.playerKey = published_player_key(session);
     snapshot.lifetime = lifetimeState;
-    // Hold the spawn gate only until the region is instantiated. World-state 8 is written after
-    // the spawn, so waiting on it here deadlocks the spawn.
-    snapshot.awaitClientSync = !client_region_ready(session, refresh);
+    // Hold the native spawn gate until the client's arrival report, WS-702 world-state 8. A
+    // region can be loaded before the bootflow arms its fade; a spawn before the arm releases an
+    // inactive fade and leaves the screen black. Arrival does not depend on the spawn (RE/30).
+    snapshot.awaitClientSync = !client_in_world(session, refresh);
     // Player_BindComponents walks every type-13 reference and the player datum can name any one of
     // them. So every participation record carries the same player key. Selecting the first slot
     // leaves the authored cinematic participant unbound whenever it names another record.

Sunrise/src/server/bap/encrypted/push/activity/internal.h

@@ -173,16 +173,17 @@ struct RefreshReport final {
 client_placement(const Session& session, const RefreshReport* refresh) noexcept;
 
 /**
- * Tests whether the client is in a live world: it holds the region it reported and no host
- * move is waiting for its arrival.
+ * Tests whether the client has reported arrival in its instantiated region: its WS-702 world
+ * state reached 8 while it holds the region it reported and no host move is waiting. This is
+ * the report that releases the native spawn gate.
  * @param session Connection whose activity session the client reports on.
  * @param refresh Refresh being answered, or null.
  */
 [[nodiscard]] bool client_in_world(const Session& session, const RefreshReport* refresh) noexcept;
 
 /**
- * Tests whether the client's destination region is instantiated far enough for the native spawn.
- * Never reads the post-spawn world-state 8 write-back; that would be a circular wait.
+ * Tests whether the client's destination region is instantiated, before its arrival report.
+ * This advances the loading lifetime. The spawn gate itself waits for `client_in_world`.
  * @param session Connection whose activity session the client reports on.
  * @param refresh Refresh being answered, or null.
  */

Sunrise/src/server/gameplay/peer/peer_out_of_band.cpp

@@ -29,11 +29,17 @@ constexpr unsigned kByteBits = 8;
 /** Sequence the first packet to a peer carries, because the head advances before it is written. */
 constexpr std::uint16_t kFirstPacketSequence = 1;
 
-/** Fills the address blob that names this host on the direct path. */
-void local_address(std::array<std::byte, wire::kAddressBlobSize>& output) noexcept {
+/**
+ * Fills the address blob that names this host on the direct path.
+ * @param receivingPort Host pool port the request arrived on. Zero names the primary port, as it
+ * does on the transport's send path.
+ * @param output Receives the direct-path address blob.
+ */
+void local_address(std::uint16_t receivingPort,
+                   std::array<std::byte, wire::kAddressBlobSize>& output) noexcept {
     const gp::Endpoint advertised = endpoint::advertised();
     middleware::gameplay::descriptor::write_direct_net_addr(
-        advertised.address, advertised.port, output);
+        advertised.address, receivingPort != 0 ? receivingPort : advertised.port, output);
 }
 
 /** @return A random 32-bit sequence, or zero when Windows refused. */
@@ -98,7 +104,9 @@ void answer_connect(const gp::Endpoint& from,
     // The peer checks both echoed fields and closes the connection on a wrong sequence.
     response.remoteChannelId = request.channelId;
     response.remoteSequence = request.sequence;
-    local_address(response.address);
+    // The client locates its connecting channel by this address, so it must name the host pool
+    // port the request reached rather than always the primary port.
+    local_address(from.localPort, response.address);
     DisplacedExternals displaced{};
     std::size_t displacedCount = 0;
     std::array<std::uint64_t, gp::kSessionsPerLink> resetSessions{};