Create and delete characters inside the investment transaction

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Sunrise/src/server/bap/encrypted/body/bap_service_body.cpp

@@ -12,14 +12,13 @@
 #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"
 #include "../../../gameplay/group/group_host_sessions.h"
 #include "../../../web_service/opcode_routes.h"
 #include "../../../web_service/web_service_runtime.h"
+#include "../../runtime.h"
 #include "../activity_host_manager/activity_host_manager_route.h"
 #include "../activity_message/activity_message_route.h"
 #include "../internal.h"
@@ -207,108 +206,6 @@ 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 the client is still on the character-select step. Nothing
-            // here may move the selection: a selected character is a pick, and a pick is what
-            // leaves the screen for orbit, so the delete would skip the roster it just changed.
-            // The account object the armed resync re-encodes already carries the new roster count,
-            // so the reply only has to name the revision that resync will publish -- the same
-            // "name the following resync" contract the claim path below uses -- and the step waits
-            // on it instead of walking on. Left at its default the field reads as revision zero,
-            // which the wait blocks on, and the roster screen never finishes the delete it asked
-            // for.
-            std::int32_t deletionVersion = middleware::web_service::kNoFamily4Publication;
-            if (queuezState.family4Active
-                && queuezState.family4Version != (std::numeric_limits<std::int32_t>::max)()) {
-                deletionVersion = queuezState.family4Version + 1;
-            }
-            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,
-                                            creation.appearanceHeader,
-                                            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();
-            // The new character joins the roster unselected (see create_character), and the client
-            // is still on the character-select step. Selecting it here would be a pick the player
-            // never made, and a pick is what leaves the step for orbit, so creation would skip the
-            // roster it just changed. The armed resync re-encodes the account object with the new
-            // roster count, so the reply only names the revision that resync will publish and the
-            // step waits on it. A change-character patch cannot do this: 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.
-            std::int32_t creationVersion = middleware::web_service::kNoFamily4Publication;
-            if (queuezState.family4Active
-                && queuezState.family4Version != (std::numeric_limits<std::int32_t>::max)()) {
-                creationVersion = queuezState.family4Version + 1;
-            }
-            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;
@@ -338,6 +235,24 @@ bool process(const ServiceRoute& route,
             }
             sunrise::server::bap::arm_account_resync_everywhere();
         }
+        if (webOutcome.hasRosterChange) {
+            // Name the revision the account resync below publishes; it carries the new roster.
+            if (queuezState.family4Active
+                && queuezState.family4Version != (std::numeric_limits<std::int32_t>::max)()
+                && !sunrise::server::web_service::encode_roster_reply(
+                    message, queuezState.family4Version + 1, webOutcome, output, written)) {
+                core::log::write(core::log::Channel::server,
+                                 core::log::Level::warn,
+                                 "ev=roster stage=response result=fail");
+                // Returning before the commit drops the change with this transaction.
+                return false;
+            }
+            sunrise::server::bap::arm_account_resync_everywhere();
+            if (webOutcome.createdCharacterSoid != 0) {
+                // One more refresh slice builds the ability rows of the new character's subclass.
+                sunrise::server::bap::request_investment_slice();
+            }
+        }
         outcome.hasSubscription = webOutcome.hasSubscription;
         outcome.hasRecordClaim = webOutcome.hasRecordClaim;
         outcome.hasArtifactReset = webOutcome.hasArtifactReset;

Sunrise/src/server/web_service/web_service_actions.cpp

@@ -9,6 +9,8 @@
 #include "../../middleware/web_service/messages/opcode402.h"
 #include "../../middleware/web_service/messages/opcode403.h"
 #include "../../middleware/web_service/messages/opcode406.h"
+#include "../../middleware/web_service/messages/opcode501_request_codec.h"
+#include "../../middleware/web_service/messages/opcode502.h"
 #include "../../middleware/web_service/messages/opcode504.h"
 #include "../../middleware/web_service/messages/opcode701/opcode701_codec.h"
 #include "../../middleware/web_service/messages/opcode801.h"
@@ -116,6 +118,56 @@ void select_character(const middleware::web_service::Message& message, Outcome&
     outcome.selectedCharacterSoid = picked.characterSoid;
 }
 
+/**
+ * Saves the character an opcode-501 request describes, with its starter set and subclasses.
+ * Both writes share one save transaction, so a refusal leaves no trace.
+ * @param message Parsed create-character request.
+ * @param outcome Gets the roster change and the new id once both writes commit.
+ */
+void create_character(const middleware::web_service::Message& message, Outcome& outcome) noexcept {
+    namespace opcode501 = middleware::web_service::messages::opcode501;
+    opcode501::Request request{};
+    if (!opcode501::parse_request(message, request)) {
+        core::log::write(
+            core::log::Channel::server, core::log::Level::warn, "ev=ws501 stage=parse result=fail");
+        return;
+    }
+    state::investment::store::Transaction transaction;
+    std::uint64_t characterSoid = 0;
+    if (!transaction.ready()
+        || !state::create_character(request.characterClass,
+                                    request.gender,
+                                    request.race,
+                                    request.appearanceHeader,
+                                    characterSoid)
+        || !state::ensure_character_subclasses() || !transaction.commit()) {
+        core::log::write(core::log::Channel::server,
+                         core::log::Level::warn,
+                         "ev=ws501 stage=create result=fail");
+        return;
+    }
+    outcome.hasRosterChange = true;
+    outcome.createdCharacterSoid = characterSoid;
+}
+
+/**
+ * Removes the character an opcode-502 request names.
+ * @param message Parsed delete-character request.
+ * @param outcome Gets the roster change once the removal commits.
+ */
+void delete_character(const middleware::web_service::Message& message, Outcome& outcome) noexcept {
+    namespace opcode502 = middleware::web_service::messages::opcode502;
+    opcode502::Request request{};
+    if (!opcode502::parse_request(message, request)
+        || !state::delete_character(request.characterSoid)) {
+        core::log::write(core::log::Channel::server,
+                         core::log::Level::warn,
+                         "ev=ws502 stage=delete result=fail");
+        return;
+    }
+    outcome.hasRosterChange = true;
+}
+
 /** Reads the shared opcode-403/404 SOID descriptor through its codec. */
 [[nodiscard]] bool parse_equipment_instance(const middleware::web_service::Message& message,
                                             std::uint64_t& instanceSoid) noexcept {

Sunrise/src/server/web_service/web_service_actions.h

@@ -8,6 +8,10 @@
 namespace sunrise::server::web_service {
 
 void select_character(const middleware::web_service::Message& message, Outcome& outcome) noexcept;
+/** Saves the character an opcode-501 request describes, with its starter set. */
+void create_character(const middleware::web_service::Message& message, Outcome& outcome) noexcept;
+/** Removes the character an opcode-502 request names. */
+void delete_character(const middleware::web_service::Message& message, Outcome& outcome) noexcept;
 void mutate_equipment(const middleware::web_service::Message& message,
                       bool unequip,
                       Outcome& outcome) noexcept;

Sunrise/src/server/web_service/web_service_runtime.cpp

@@ -14,6 +14,7 @@
 #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/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"
@@ -279,6 +280,36 @@ bool encode_resident_dependent_refusal(std::span<const std::byte> request,
            || encode_echo(message, response, written);
 }
 
+/**
+ * Encodes the reply to an opcode-501 create or 502 delete, a refusal when nothing changed.
+ * @param message Parsed request; its opcode picks the reply shape.
+ * @param family4Version Revision carrying the change, or kNoFamily4Publication.
+ * @param outcome The roster change and created id, empty for a refusal.
+ * @param response Svc-11 response-body storage owned by the caller.
+ * @param written Gets the encoded response-body size.
+ * @return False when the reply does not fit.
+ */
+bool encode_roster_reply(const middleware::web_service::Message& message,
+                         std::int32_t family4Version,
+                         const Outcome& outcome,
+                         std::span<std::byte> response,
+                         std::size_t& written) noexcept {
+    namespace ws = middleware::web_service;
+    ws::StatusResponse status{};
+    status.value = family4Version;
+    if (!outcome.hasRosterChange) {
+        // A refusal stages nothing, so the version wait must not wait.
+        status.code = kRefusedStatus;
+        status.value = ws::kNoFamily4Publication;
+    }
+    if (message.opcode == ws::messages::opcode501::kOpcode) {
+        // The 501 shape always carries the id field, zero on a refusal.
+        return ws::messages::opcode501::encode_response(
+            message, status, outcome.createdCharacterSoid, response, written);
+    }
+    return ws::encode_response(message, ws::ResponseShape::statusPair, status, response, written);
+}
+
 /**
  * Parses one request, prepares any action it names, and encodes the reply that reports it.
  * @param request Whole decrypted svc-10 body.
@@ -286,7 +317,8 @@ bool encode_resident_dependent_refusal(std::span<const std::byte> request,
  * @param written Gets the encoded response-body size, or zero when the header does not parse.
  * @param outcome Gets the prepared action for the caller to publish, and is left empty when
  * the action was refused or the reply could not be encoded.
- * @return False only when the envelope header does not parse.
+ * @return False when the envelope header does not parse, or a saved roster change has no
+ *         reply; the caller's save transaction must then drop the change.
  */
 bool consume(std::span<const std::byte> request,
              std::span<std::byte> response,
@@ -336,16 +368,23 @@ bool consume(std::span<const std::byte> request,
         return true;
     }
 
-    if (message.opcode == middleware::web_service::messages::opcode501::kOpcode) {
-        // 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, status, characterSoid, response, written)
-               || encode_echo(message, response, written);
+    if (message.opcode == middleware::web_service::messages::opcode501::kOpcode
+        || message.opcode == middleware::web_service::messages::opcode502::kOpcode) {
+        if (message.opcode == middleware::web_service::messages::opcode501::kOpcode) {
+            create_character(message, outcome);
+        } else {
+            delete_character(message, outcome);
+        }
+        // The caller re-encodes a change with the revision that publishes it.
+        if (encode_roster_reply(message,
+                                middleware::web_service::kNoFamily4Publication,
+                                outcome,
+                                response,
+                                written)) {
+            return true;
+        }
+        // A saved change must not outlive a missing reply; false makes the caller drop it.
+        return !outcome.hasRosterChange && encode_echo(message, response, written);
     }
 
     // The artifact vendor is answered here. Every other vendor purchase falls through to the

Sunrise/src/server/web_service/web_service_runtime.h

@@ -10,6 +10,7 @@
 #include <variant>
 
 #include "../../middleware/web_service/messages/opcode206.h"
+#include "../../middleware/web_service/web_service_envelope.h"
 #include "../../state/account/inventory/seen_state.h"
 #include "../../state/runtime/runtime.h"
 
@@ -26,6 +27,10 @@ struct Outcome {
     /** An opcode-504 pick was recorded and its Family-4 update still has to follow. */
     bool hasSelectedCharacter{};
     std::uint64_t selectedCharacterSoid{};
+    /** An opcode-501 create or 502 delete saved a roster change; the account resync follows. */
+    bool hasRosterChange{};
+    /** The character an opcode-501 create added, which its reply names. */
+    std::uint64_t createdCharacterSoid{};
     /** Reset is precommitted because it changes persistence and account currency together. */
     bool hasArtifactReset{};
     state::ArtifactResetResult artifactReset{};
@@ -109,7 +114,8 @@ inline void clear_mutation(Outcome& outcome) noexcept {
  * @param written Gets the encoded response-body size, or zero when the header does not parse.
  * @param outcome Gets the prepared action for the caller to publish, and is left empty when
  * the action was refused or the reply could not be encoded.
- * @return False only when the envelope header does not parse.
+ * @return False when the envelope header does not parse, or a saved roster change has no
+ *         reply; the caller's save transaction must then drop the change.
  */
 [[nodiscard]] bool
 consume(std::span<const std::byte> request,
@@ -118,6 +124,21 @@ consume(std::span<const std::byte> request,
         Outcome& outcome,
         std::span<const state::account::inventory::PresentedItemRow> presentation = {}) noexcept;
 
+/**
+ * Encodes the reply to an opcode-501 create or 502 delete, a refusal when nothing changed.
+ * @param message Parsed request; its opcode picks the reply shape.
+ * @param family4Version Revision carrying the change, or kNoFamily4Publication.
+ * @param outcome The roster change and created id, empty for a refusal.
+ * @param response Svc-11 response-body storage owned by the caller.
+ * @param written Gets the encoded response-body size.
+ * @return False when the reply does not fit.
+ */
+[[nodiscard]] bool encode_roster_reply(const middleware::web_service::Message& message,
+                                       std::int32_t family4Version,
+                                       const Outcome& outcome,
+                                       std::span<std::byte> response,
+                                       std::size_t& written) noexcept;
+
 /** Encodes the normal refusal shape for a request that may publish resident references. */
 [[nodiscard]] bool encode_resident_dependent_refusal(std::span<const std::byte> request,
                                                      std::span<std::byte> response,