ソースを参照

fix: stage the Family-4 select-character publish for create/delete

The real client blocks after opcode 501/502 waiting for a Family-4
publish at the version the response names. The stock opcode501 codec
hardcoded that value to kNoFamily4Publication (correct for its original
"echo the selected character" use, wrong once the request actually
creates or deletes a character), and opcode502 answered with a bare
echo with no version at all. Both left the client waiting on a
publication that was never staged, which times out into a disconnect.

Moved the actual opcode 501/502 handling into bap_service_body.cpp,
alongside the existing opcode 505 change-character block it mirrors:
stage a queuez::SelectCharacter transaction onto the new/surviving
character, encode the real staged Family-4 version in the response,
and arm a full account resync as a safety net if staging fails or the
frame gets dropped. opcode501_codec::encode_response now takes the
status pair as a parameter instead of hardcoding it, so the existing
stock call site (character-select echo) keeps its original behavior
explicitly instead of implicitly.
T4tsuk3i 1 ヶ月 前
親
コミット
7473278539

+ 7 - 5
Sunrise/src/middleware/web_service/messages/opcode501_codec.cpp

@@ -11,8 +11,9 @@ constexpr std::uint8_t kCharacterSoidWidth = 64;
 
 } // namespace
 
-/** Writes the echoed header, the shared status pair, the character id, and the trailer. */
+/** Writes the echoed header, the caller's status pair, the character id, and the trailer. */
 bool encode_response(const Message& message,
+                     const StatusResponse& status,
                      std::uint64_t characterSoid,
                      std::span<std::byte> output,
                      std::size_t& written) noexcept {
@@ -20,10 +21,11 @@ bool encode_response(const Message& message,
     if (output.size() < kEnvelopeHeaderSize) {
         return false;
     }
-    encoding::bits::Writer writer = begin_response(message, output);
-    // The reply names a character the roster already holds, so no revision publishes it.
-    StatusResponse status{};
-    status.value = kNoFamily4Publication;
+    encoding::write_u16_be(output.first<encoding::kU16Size>(), message.opcode);
+    encoding::write_u32_be(output.subspan<encoding::kU16Size, encoding::kU32Size>(),
+                           message.transactionId);
+
+    encoding::bits::Writer writer(output.subspan(kEnvelopeHeaderSize));
     // The id carries no presence bit or bias.
     bool encoded = status::write_fields(writer, ResponseShape::statusPair, status)
                    && writer.write(characterSoid, kCharacterSoidWidth)

+ 7 - 1
Sunrise/src/middleware/web_service/messages/opcode501_codec.h

@@ -13,13 +13,19 @@ inline constexpr std::uint16_t kOpcode = 501;
 
 /**
  * Encodes the create-character response: the status pair then the character object id.
+ * The status is the caller's, because this opcode has no single right answer. A refusal has to
+ * say so in the code -- the SOID field alone cannot, since zero is what a refusal would send and
+ * the Client reads the code, not the id -- and the value feeds the Family-4 version wait, which
+ * needs the revision a staged publication will carry, or kNoFamily4Publication when none will.
  * @param message Parsed request whose envelope fields are echoed.
- * @param characterSoid Character object id; must be published in family 3.
+ * @param status Logical status pair: refusal code, and the revision the wait should expect.
+ * @param characterSoid Character object id; must be published in family 3. Zero on a refusal.
  * @param output Caller-owned svc-11 response-body storage.
  * @param written Receives encoded response-body bytes.
  * @return True when the fixed response fits the output buffer.
  */
 [[nodiscard]] bool encode_response(const Message& message,
+                                   const StatusResponse& status,
                                    std::uint64_t characterSoid,
                                    std::span<std::byte> output,
                                    std::size_t& written) noexcept;

+ 107 - 0
Sunrise/src/server/bap/encrypted/body/bap_service_body.cpp

@@ -12,6 +12,8 @@
 #include "../../../../middleware/bap/family_unsubscription.h"
 #include "../../../../middleware/bap/user_message/user_message_response.h"
 #include "../../../../middleware/encoding/byte_order.h"
+#include "../../../../middleware/web_service/messages/opcode501_request_codec.h"
+#include "../../../../middleware/web_service/messages/opcode502.h"
 #include "../../../../middleware/web_service/messages/opcode505/opcode505_codec.h"
 #include "../../../../state/activity/membership/activity_membership_query.h"
 #include "../../../../state/runtime/runtime.h"
@@ -205,6 +207,111 @@ bool process(const ServiceRoute& route,
             outcome.hasChangeCharacter = true;
             return true;
         }
+        // Character deletion. A refusal still answers with the status pair: the roster screen
+        // waits on the echoed transaction id, so a missing reply hangs it for the rest of the run.
+        if (middleware::web_service::parse_request(requestBody, message)
+            && message.opcode == middleware::web_service::messages::opcode502::kOpcode) {
+            const auto refuse = [&]() noexcept {
+                middleware::web_service::StatusResponse status{};
+                status.code = middleware::web_service::kRefusedStatusCode;
+                // A refused delete stages nothing, so the version wait must be told not to wait.
+                status.value = middleware::web_service::kNoFamily4Publication;
+                return middleware::web_service::encode_response(
+                    message,
+                    middleware::web_service::ResponseShape::statusPair,
+                    status,
+                    output,
+                    written);
+            };
+            middleware::web_service::messages::opcode502::Request deletion{};
+            if (!middleware::web_service::messages::opcode502::parse_request(message, deletion)
+                || deletion.characterSoid == 0
+                || !state::delete_character(deletion.characterSoid)) {
+                core::log::write(core::log::Channel::server,
+                                 core::log::Level::warn,
+                                 "ev=ws502 stage=delete result=fail");
+                return refuse();
+            }
+            // The account is already changed and written (delete_character persists through the
+            // investment store internally), but the caller can still drop the frame afterwards --
+            // a stale entity lease, a refused commit, or a response that does not fit -- and the
+            // outcome flag at the end of this file only lands on the handled path. Without this
+            // the character is gone from disk while the client keeps a roster that names it.
+            sunrise::server::bap::arm_account_resync_everywhere();
+            // The roster shrank and delete_character has already moved the selection onto a
+            // survivor, so the client needs the Family-4 character object moved with it. Opcode
+            // 502 feeds the Client's Family-4 version wait: the reply has to name the revision the
+            // wait should expect, the staged one when a publication is coming and the no-wait
+            // constant when none is -- left at its default the field reads as revision zero, which
+            // the wait blocks on, and the roster screen never finishes the delete it just asked for.
+            std::int32_t deletionVersion = middleware::web_service::kNoFamily4Publication;
+            const std::uint64_t survivorSoid =
+                state::account::selected_character_soid(state::account_snapshot());
+            auto* deletionSelect = emplace_transaction<queuez::SelectCharacter>(outcome);
+            if (survivorSoid != 0 && deletionSelect != nullptr
+                && queuez::stage_select_character(queuezState, survivorSoid, *deletionSelect)) {
+                outcome.hasSelectCharacter = true;
+                deletionVersion = deletionSelect->after.family4Version;
+            } else {
+                clear_transaction(outcome);
+                sunrise::server::bap::arm_account_resync_everywhere();
+            }
+            core::log::write(
+                core::log::Channel::server, core::log::Level::info, "ev=ws502 stage=delete result=ok");
+            middleware::web_service::StatusResponse deletionStatus{};
+            deletionStatus.value = deletionVersion;
+            return middleware::web_service::encode_response(
+                message,
+                middleware::web_service::ResponseShape::statusPair,
+                deletionStatus,
+                output,
+                written);
+        }
+        // Character creation. Both outcomes answer in opcode 501's own response shape, which
+        // carries the new SOID: the generic status pair omits that field and the Client fatally
+        // disconnects trying to decode it.
+        if (middleware::web_service::parse_request(requestBody, message)
+            && message.opcode == middleware::web_service::messages::opcode501::kOpcode) {
+            middleware::web_service::StatusResponse creationRefusal{};
+            creationRefusal.code = middleware::web_service::kRefusedStatusCode;
+            creationRefusal.value = middleware::web_service::kNoFamily4Publication;
+            middleware::web_service::messages::opcode501::Request creation{};
+            std::uint64_t characterSoid = 0;
+            if (!middleware::web_service::messages::opcode501::parse_request(message, creation)
+                || !state::create_character(
+                    creation.characterClass, creation.gender, creation.race, characterSoid)) {
+                core::log::write(core::log::Channel::server,
+                                 core::log::Level::warn,
+                                 "ev=ws501 stage=create result=fail");
+                return middleware::web_service::messages::opcode501::encode_response(
+                    message, creationRefusal, 0, output, written);
+            }
+            // As in the delete branch: the account is already changed and written, but the caller
+            // can still drop the frame, and the outcome flag only lands on the handled path.
+            sunrise::server::bap::arm_account_resync_everywhere();
+            // create_character marks the new character selected, and the client is waiting for the
+            // Family-4 character object to move onto it. A change-character patch cannot do that
+            // here: it refuses unless family three is in its normal phase, and the creation flow
+            // leaves it outside that phase, so the client would wait forever on a publication that
+            // was never staged. Selecting is both ungated and what the client actually asked for.
+            std::int32_t creationVersion = middleware::web_service::kNoFamily4Publication;
+            auto* creationSelect = emplace_transaction<queuez::SelectCharacter>(outcome);
+            if (creationSelect != nullptr
+                && queuez::stage_select_character(queuezState, characterSoid, *creationSelect)) {
+                outcome.hasSelectCharacter = true;
+                creationVersion = creationSelect->after.family4Version;
+            } else {
+                clear_transaction(outcome);
+                sunrise::server::bap::arm_account_resync_everywhere();
+            }
+            core::log::write(core::log::Channel::server,
+                             core::log::Level::info,
+                             "ev=ws501 stage=create result=ok");
+            middleware::web_service::StatusResponse creationStatus{};
+            creationStatus.value = creationVersion;
+            return middleware::web_service::messages::opcode501::encode_response(
+                message, creationStatus, characterSoid, output, written);
+        }
         state::investment::store::Transaction investmentTransaction;
         if (!investmentTransaction.ready()) {
             return false;

+ 7 - 24
Sunrise/src/server/web_service/web_service_runtime.cpp

@@ -14,8 +14,6 @@
 #include "../../middleware/web_service/messages/opcode206.h"
 #include "../../middleware/web_service/messages/opcode2400.h"
 #include "../../middleware/web_service/messages/opcode501_codec.h"
-#include "../../middleware/web_service/messages/opcode501_request_codec.h"
-#include "../../middleware/web_service/messages/opcode502.h"
 #include "../../middleware/web_service/messages/opcode503.h"
 #include "../../middleware/web_service/messages/opcode504.h"
 #include "../../middleware/web_service/messages/opcode601/opcode601_codec.h"
@@ -339,32 +337,17 @@ bool consume(std::span<const std::byte> request,
     }
 
     if (message.opcode == middleware::web_service::messages::opcode501::kOpcode) {
-        middleware::web_service::messages::opcode501::Request created{};
-        std::uint64_t characterSoid = 0;
-        if (!middleware::web_service::messages::opcode501::parse_request(message, created)
-            || !state::create_character(
-                created.characterClass, created.gender, created.race, characterSoid)) {
-            core::log::write(
-                core::log::Channel::server, core::log::Level::warn, "ev=ws501 stage=create result=fail");
-            // The client still needs an answer even when creation fails; falling back to the
-            // selected character keeps the response shape valid instead of leaving it unanswered.
-            characterSoid = state::account::selected_character_soid(state::account_snapshot());
-        }
+        // Returns a SOID family three already publishes. The request body is not parsed.
+        const std::uint64_t characterSoid =
+            state::account::selected_character_soid(state::account_snapshot());
+        // The reply names a character the roster already holds, so no revision publishes it.
+        middleware::web_service::StatusResponse status{};
+        status.value = middleware::web_service::kNoFamily4Publication;
         return middleware::web_service::messages::opcode501::encode_response(
-                   message, characterSoid, response, written)
+                   message, status, characterSoid, response, written)
                || encode_echo(message, response, written);
     }
 
-    if (message.opcode == middleware::web_service::messages::opcode502::kOpcode) {
-        middleware::web_service::messages::opcode502::Request targeted{};
-        if (!middleware::web_service::messages::opcode502::parse_request(message, targeted)
-            || !state::delete_character(targeted.characterSoid)) {
-            core::log::write(
-                core::log::Channel::server, core::log::Level::warn, "ev=ws502 stage=delete result=fail");
-        }
-        return encode_echo(message, response, written);
-    }
-
     // The artifact vendor is answered here. Every other vendor purchase falls through to the
     // shared response-shape path, which runs the action and answers its status: an action that
     // prepared no mutation is answered with the refused code.