Skip to content

Add full achievement definition and character achievement editors - #90

Open
fryguy503 wants to merge 5 commits into
Valorith:masterfrom
fryguy503:feat/achievements
Open

Add full achievement definition and character achievement editors#90
fryguy503 wants to merge 5 commits into
Valorith:masterfrom
fryguy503:feat/achievements

Conversation

@fryguy503

@fryguy503 fryguy503 commented Aug 8, 2026

Copy link
Copy Markdown

DO NOT MERGE UNTIL EQEmu/EQEmu#5119 has been accepted

Summary

This PR adds a complete achievement-management feature to Spire, covering both:

  • Achievement definition authoring under World Data → Achievements
  • Durable character achievement administration under Player Operations → Character Achievements

The feature replaces direct SQL management with validated, transactional editors for categories, definitions, components, criteria, rewards, selectable reward sets, spell restrictions, character progress, reward ledgers, and queued achievement mutations.

All material fields include inline explanations, contextual help, or accessible descriptions so editors can understand what each value controls and how it affects the EQEmu runtime.

Motivation

Achievement definitions are stored as an interconnected graph spanning numerous tables. Editing these records manually creates significant risks:

  • Breaking stable IDs already referenced by character state
  • Publishing invalid event or reward combinations
  • Creating dependency or category cycles
  • Accidentally duplicating reward delivery
  • Overwriting another editor’s changes
  • Editing character state while a zone process still owns it
  • Losing orphaned criteria or character history
  • Misinterpreting large unsigned database values through JavaScript

This PR provides a guided authoring and support interface with server-side validation, optimistic concurrency, transaction boundaries, audit history, and explicit recovery workflows.

User-facing features

World Data → Achievements

Adds an Alpha/New Achievements entry to the World Data navigation group.

The achievement workspace contains three primary modes:

  • Definitions
  • Categories
  • Authoring Guide

Definition catalog

The definition directory supports:

  • Search by achievement ID, name, or description
  • Enabled/disabled publication filtering
  • Category filtering
  • Criterion-event filtering
  • Reward-type filtering
  • Reward-content filtering:
    • Automatic rewards
    • Selectable reward sets
    • Any rewards
    • No authored rewards
  • Stable server-side sorting
  • Bounded pagination
  • Refresh, loading, error, and empty-result states

Directory rows summarize:

  • Stable achievement ID
  • Name
  • Points
  • Definition version
  • Enabled state
  • Component count
  • Reward count

Complete definition graph editor

A definition is loaded and saved as one complete graph rather than as disconnected SQL rows.

The editor supports:

General

  • Stable achievement ID
  • Name and description
  • Client icon ID
  • Achievement points
  • Reward-display provenance
  • World-display flag
  • Definition version
  • Reset-on-version-change policy
  • Enabled/disabled publication state
  • Required audit reason

New definitions must be created disabled so they can be reviewed before publication.

Category associations

  • Multiple categories per definition
  • Category lookup
  • Per-category ordering
  • Optional association-specific display text

Components

  • Client wire type
  • Stable component ID
  • Client presentation order
  • Global presentation count
  • Primary and secondary player-facing descriptions

Component types 0–2 carry durable state. Type 3 is presentation-only and cannot contain enabled criteria.

Criteria

Each component can contain nested criteria with:

  • Criterion event
  • Progress mode
  • Completion behavior
  • Primary event target
  • Secondary event target
  • Signed BIGINT target value
  • Required count
  • Enabled state

The editor supports all achievement events currently implemented by EQEmu:

  1. Manual
  2. Level
  3. NPC Type Kill
  4. NPC Race Kill
  5. Task Complete
  6. Zone Enter
  7. Loot Item
  8. Own Item
  9. Tradeskill Success
  10. Skill Value
  11. Alternate Advancement
  12. Achievement Complete
  13. NPC Name Kill
  14. Skill Cap

Event selection dynamically changes:

  • Field labels
  • Field explanations
  • Reference lookup type
  • Allowed progress modes
  • Wildcard behavior
  • Class and skill controls
  • Secondary-target availability

Supported component behaviors include:

  • Required
  • Optional
  • Unlock
  • Visibility
  • Display Only
  • Blocker

Supported progress modes include:

  • Increment
  • Highest
  • Set
  • Boolean

Canonical skill and class values

The editor exposes:

  • Canonical EQ class IDs 1–16
  • Canonical EQEmu skill IDs 0–77
  • Corrected skill labels including Dual Wield and Make Poison
  • Skill Value wildcard support without treating skill ID 0 as a wildcard

Rewards

The editor supports canonical reward grants for:

  • Items
  • Experience
  • Alternate Advancement points
  • Copper
  • Alternate currency
  • Titles

Each reward includes:

  • Durable reward ID
  • Sequence
  • Reward type
  • Referenced data ID
  • Exact unsigned amount
  • Client description
  • Enabled state

New reward IDs are allocated transactionally by the server and returned as decimal strings to preserve unsigned BIGINT precision.

Selectable reward sets

A definition can author a selectable reward set containing:

  • Stable reward-set ID
  • Prompt/title
  • Enabled state
  • Stable option IDs
  • Option order
  • Option label
  • Common-to-all behavior
  • Client flags
  • Enabled state
  • Canonical reward-to-option mappings

New unsaved rewards use safe transient mapping tokens until the server allocates their durable reward IDs.

Validation ensures:

  • Every enabled option has an enabled mapped grant
  • An enabled set has at least one enabled non-common option
  • Common grants combine with exactly one selected non-common option
  • A canonical reward belongs to at most one option
  • Enabled rewards are not stranded behind disabled sets or options
  • Mappings correctly participate in runtime version-policy checks

Spell cast restrictions

Definitions can be connected to existing achievement-aware spell restriction IDs.

The editor supports:

  • Restriction ID
  • Required completed/incomplete state
  • Duplicate and contradictory-row validation
  • Documentation that all applicable rows sharing a restriction ID must pass

Category editor

The category workspace supports:

  • Stable category ID
  • Parent category
  • Sibling order
  • Name
  • Description
  • Exact client icon/resource string
  • Required audit reason

Safety includes:

  • Missing-parent checks
  • Self-parenting prevention
  • Category-cycle detection
  • Expected revision and parent guards
  • Delete blocking while children or definition associations remain
  • Exact typed delete confirmation

Bounded reference lookups

Reusable reference pickers are provided for:

  • Achievements
  • Categories
  • NPC types
  • Canonical NPC names
  • NPC races
  • Tasks
  • Zones
  • Items
  • Tradeskill recipes
  • Alternate currencies
  • Title sets

Lookup behavior includes:

  • Debounced searching
  • Exact numeric-ID searching
  • Minimum text-search length
  • Bounded results
  • Loading, error, and empty-result states
  • Direct numeric entry when a reviewed custom ID is required
  • ARIA listbox/option semantics

Item lookup results include the native Spire item sprite supplied by items.icon. Achievement-specific styling prevents the EQWindow button theme from collapsing the icon grid.

NPC Name Kill helper

The authoring guide and criterion editor include a helper that:

  • Canonicalizes an NPC name
  • Collapses spaces and underscores
  • Lowercases ASCII letters
  • Removes unsupported characters
  • Computes the unsigned 32-bit FNV-1a name hash

This avoids requiring authors to calculate hashes manually.

Validation and authoring education

Field metadata is supplied by the backend and consumed dynamically by the frontend.

Every material field includes:

  • A visible label
  • A brief explanation
  • Event- or reward-specific context
  • aria-describedby linkage where appropriate
  • Range, wildcard, or stable-identity guidance

The embedded authoring guide documents:

  • The author-disabled-to-published lifecycle
  • Components versus criteria
  • State-bearing versus presentation-only types
  • Progress modes
  • Replay-safe event configuration
  • Required, optional, unlock, visibility, and blocker behavior
  • Canonical rewards versus selectable options
  • Common reward options
  • Stable identities
  • Definition-version semantics
  • Reset-on-version-change consequences
  • Spell restrictions
  • Transaction and schema ownership
  • Stale-write protection
  • Every criterion event and its target meanings

Authoritative server validation

Browser validation provides immediate feedback, but backend validation remains authoritative.

Validation includes:

  • Required graph collections
  • Stable IDs and immutable ownership
  • Numeric type and wire limits
  • UTF-8 byte limits for MySQL TEXT columns
  • Duplicate IDs, sequences, associations, and mappings
  • Component wire types
  • Presentation-only component restrictions
  • All criterion event types
  • Allowed progress modes by event
  • Replay-safe progress behavior
  • Event-specific targets and wildcard rules
  • Class and skill ranges
  • Required-count consistency
  • Alternative-criterion policy consistency
  • Global component presentation-count consistency
  • Category existence and parent cycles
  • Achievement dependency existence and cycles
  • Reward reference and amount rules
  • Runtime reward-delivery limits
  • Selectable-set ownership and grant integrity
  • Cast-restriction duplication and contradictions
  • Reference existence for enabled runtime rows

Reference validation covers:

  • npc_types
  • tasks
  • zone
  • items
  • tradeskill_recipe
  • skill_caps
  • alternate_currency
  • titles.title_set

Missing references or unavailable reference catalogs block enabled publication. Disabled drafts retain warnings so incomplete content can be safely staged.

Custom NPC race IDs remain advisory because custom engine race values may intentionally have no current npc_types row.

Graph and request limits

The API enforces:

  • Graph payload: 2 MiB
  • Smaller mutation payloads: 128 KiB
  • Category associations: 100
  • Components: 1,000
  • Criteria: 2,000
  • Rewards: 500
  • Cast restrictions: 500
  • Reward options: 500
  • Reward mappings: 500
  • Lookup results: 100
  • Standard result pages: maximum 200 rows

Large numeric identities and values are represented as decimal strings where JavaScript Number would lose precision.

Transaction and concurrency safety

Definition and category writes use:

  • Explicit database transactions
  • MySQL advisory authoring lock
  • FOR UPDATE row locks
  • Complete graph validation inside the transaction
  • Expected SHA-256 graph/category revision
  • Stale-write 409 Conflict responses
  • Rollback on any failure

Runtime-policy fingerprints distinguish presentation-only changes from changes that can reinterpret durable player state.

A definition-version increase is required when changing runtime evaluation, reward, mapping, or reset policy, even if the definition is temporarily disabled before being re-enabled.

Presentation-only edits do not require unnecessary version bumps.

Stable identity protection

The editor prevents unsafe identity changes after content has been deployed.

Protected identities include:

  • Achievement IDs
  • Category IDs
  • Component type/ID pairs
  • Canonical reward IDs
  • Reward-set IDs
  • Reward-option IDs

Persisted identities generally must be disabled instead of removed or renumbered.

Creating or cloning an achievement also checks durable character tables before accepting the destination ID. A deleted achievement ID cannot be reused while completion, progress, reward, selection, or pending-mutation history still references it.

Clone behavior

Cloning a definition:

  • Requires a destination ID
  • Requires exact CLONE <source-id> confirmation
  • Requires the source revision to remain current
  • Produces a disabled definition
  • Resets the clone to definition version 1
  • Allocates fresh reward identities
  • Allocates a fresh reward-set identity
  • Preserves selectable/common option semantics
  • Clears cast restrictions so environment-specific restrictions must be reviewed

Definition deletion behavior

Deleting a definition:

  • Requires an audit reason
  • Requires exact DELETE <id> confirmation
  • Requires the expected graph revision
  • Is blocked when another achievement depends on it
  • Removes the authored content graph transactionally
  • Preserves global component presentation-count rows
  • Preserves durable character history

Deleted-definition character state remains visible as orphaned state in the character achievement editor.

Orphan criterion recovery

If criteria exist without their owning achievement_components row, the editor does not silently hide or delete them.

Instead it renders a recovery-only component and requires one explicit whole-group action:

  • Restore the missing component and preserve every criterion
  • Delete the complete orphan criterion group

The server rejects:

  • Partial criterion omission
  • Moving orphan criteria to another component
  • Adding rows to a recovery group
  • Clearing recovery metadata
  • Saving without an explicit recovery decision

Character Achievement administration

Adds a separately permissioned administrator workspace under:

Player Operations → Character Achievements

The character directory supports:

  • Search by exact character ID or name
  • All, Online, and Offline presence filters
  • Pagination
  • Completion count
  • Active-progress count
  • Progress-row count
  • Exact aggregate progress total
  • Last-login details

Content and character-state databases are queried independently and assembled without cross-database joins.

Character state filters

Achievement state can be filtered by:

  • All
  • Completed
  • Not completed
  • In progress
  • Not started
  • Definition version mismatch
  • Reward attention
  • Pending mutation
  • Orphaned state

Search includes:

  • Achievement ID
  • Name
  • Description
  • Associated category name
  • Association display text

Achievement state details

Expanded definition cards show:

  • Definition ID and version
  • Enabled state
  • Name and description
  • Points
  • Categories
  • Component and reward counts
  • Completion timestamp and stored version
  • Component progress
  • Reward ledgers
  • Reward selections
  • Pending mutations
  • Version mismatch warnings
  • Deleted-definition/orphan diagnostics

Character workspace tabs

The character editor contains:

  • Achievements
  • Rewards & Selections
  • Pending Queue
  • Audit & Safety

The diagnostics tabs expose:

  • Individual reward ledgers
  • Whole selectable-bundle ledgers
  • Pending, blocked, and processing mutations
  • Expired processing leases
  • Orphaned content state
  • Schema readiness
  • Status glossary
  • Paginated operator audit history

Character repair operations

All durable character mutations are intentionally offline-only.

Set exact progress

Allows an administrator to set one reviewed component count.

Safety includes:

  • State-bearing component types only
  • Enabled definition and criterion policy
  • Current requirement bounds
  • No progress beyond the authored requirement
  • No progress changes after achievement completion
  • Exact unsigned 64-bit expected-current-count comparison
  • Definition-version validation

This operation does not synthesize a game event.

Force completion

Persists a completion row at the current definition version.

It does not directly:

  • Deliver rewards
  • Synthesize dependency events
  • Pretend that game-event processing occurred

The game server remains responsible for runtime reconciliation.

Reset achievement state

The normal reset removes:

  • Completion
  • Component progress
  • Pending mutations

Reward and selection history is preserved by default to prevent duplicate delivery after recompletion.

Deleting reward history requires:

  • A separate explicit option
  • RESET REWARDS <achievement-id> confirmation
  • Regrant-risk acknowledgement

Reset also supports orphaned character state whose definition was deleted.

Retry individual reward

Eligible automatic reward ledgers can be returned to retryable state.

The editor:

  • Never grants the reward directly
  • Rejects already granted rows
  • Rejects unknown statuses
  • Requires expected status
  • Requires duplicate-delivery-risk acknowledgement
  • Refuses individual retry for selectable rewards

Selectable grants must be repaired through their owning selection bundle.

Retry selectable reward bundle

Selection retry:

  • Validates the enabled reward set
  • Validates the selected non-common option
  • Includes enabled common options
  • Verifies every mapping and reward
  • Preserves already durably granted entries
  • Moves eligible pending entries and the selection to retryable state atomically
  • Requires duplicate-risk acknowledgement

Retry pending mutation

Only compatible blocked rows can return to pending.

Retry requires:

  • Existing enabled definition
  • Nonzero queued definition version
  • Exact current definition-version match
  • Expected status
  • Expected attempt count

Legacy, missing, disabled, or version-incompatible rows remain blocked.

Discard pending mutation

Pending and blocked mutations can be discarded after review.

Active processing rows remain locked for 60 seconds. Once the lease is stale, discard or reset requires a separate stale-processing-lease acknowledgement.

Character mutation safety

Every character mutation requires:

  • Character offline
  • Non-deleted character
  • Required audit reason between 8 and 240 characters
  • Exact character-name confirmation
  • Operation-specific confirmation phrase
  • Expected count/version/status tokens
  • Per-character MySQL advisory lock
  • character_data row lock
  • Character database transaction

Content-dependent mutations also hold the achievement authoring lock while reading runtime policy and committing the character mutation. This prevents a concurrent content save from invalidating the policy mid-operation.

Online characters remain inspectable, but repair buttons are disabled because the zone process owns cached runtime state.

Split-database support

The implementation deliberately separates:

  • Achievement content through eqemu_content
  • Character state through the normal EQEmu character database
  • Operator audit records through Spire’s audit database

No content/character cross-database join is required, so separate hosts and connections are supported.

Permissions

Adds two independent permission resources:

  • Achievement Editorachievement-editor
  • Character Achievementscharacter-achievement-editor

This allows content authors to manage definitions without automatically granting access to durable player-state repair operations.

Audit history

Every successful definition, category, or character mutation creates a Spire user-event audit record.

Audited operations include:

  • Definition create
  • Definition update
  • Definition clone
  • Definition delete
  • Category create
  • Category update
  • Category delete
  • Character progress set
  • Character force complete
  • Character reset
  • Reward retry
  • Selection retry
  • Pending mutation retry
  • Pending mutation discard

Audit records include operator attribution, reason, target identity, and before/after context.

The provisional audit record is discarded when the corresponding mutation fails or rolls back.

API endpoints

Achievement definition editor

Method Endpoint
GET /api/v1/achievement-editor/metadata
GET /api/v1/achievement-editor/schema
GET /api/v1/achievement-editor/definitions
GET /api/v1/achievement-editor/definition/:id
PUT /api/v1/achievement-editor/definition
PATCH /api/v1/achievement-editor/definition/:id
PUT /api/v1/achievement-editor/definition/:id/clone
DELETE /api/v1/achievement-editor/definition/:id
GET /api/v1/achievement-editor/categories
GET /api/v1/achievement-editor/category/:id
PUT /api/v1/achievement-editor/category
PATCH /api/v1/achievement-editor/category/:id
DELETE /api/v1/achievement-editor/category/:id
GET /api/v1/achievement-editor/lookups/:kind
GET /api/v1/achievement-editor/audit

Character Achievement administration

Method Endpoint
GET /api/v1/character-achievement-editor/metadata
GET /api/v1/character-achievement-editor/schema
GET /api/v1/character-achievement-editor/characters
GET /api/v1/character-achievement-editor/character/:id
GET /api/v1/character-achievement-editor/character/:id/audit
PATCH /api/v1/character-achievement-editor/character/:id/progress
PATCH /api/v1/character-achievement-editor/character/:id/complete
PATCH /api/v1/character-achievement-editor/character/:id/reset
PATCH /api/v1/character-achievement-editor/character/:id/reward/retry
PATCH /api/v1/character-achievement-editor/character/:id/selection/retry
PATCH /api/v1/character-achievement-editor/character/:id/mutation/retry
DELETE /api/v1/character-achievement-editor/character/:id/mutation

Schema prerequisites

This PR does not create or migrate the achievement schema.

The matching EQEmu achievement migrations must already be installed.

Required content tables

achievement_categories
achievements
achievement_category_associations
achievement_components
achievement_component_counts
achievement_criteria
achievement_rewards
achievement_cast_restrictions
achievement_reward_sets
achievement_reward_options
achievement_reward_option_entries

Required character-state tables

character_data
character_achievements
character_achievement_progress
character_achievement_rewards
character_achievement_reward_selections
character_achievement_pending_mutations

The schema probe validates:

  • Required tables
  • Required columns
  • Integer base types
  • Signedness
  • Nullability
  • Auto-increment identities
  • Required unique/supporting indexes
  • InnoDB transaction and row-lock support

The feature fails closed when the schema is incomplete or incompatible. Diagnostics are cached briefly and can be explicitly rechecked from the UI.

Accessibility and responsive behavior

The editors include:

  • Semantic labels
  • Unique control IDs
  • Inline field descriptions
  • ARIA-described controls
  • Alert and status regions
  • Listbox and option semantics
  • Selected and expanded state
  • Accessible pagination labels
  • Meaningful table headers
  • Contextual disabled-action explanations
  • Keyboard focus styling
  • Ctrl/Cmd+S save shortcut
  • Unsaved-change warnings

The definition workspace progressively collapses its directory, forms, guide cards, and graph grids for smaller displays.

The character workspace reflows cards, actions, safety panels, and confirmation dialogs while retaining horizontal scrolling for large diagnostic tables.

Automated coverage

This PR adds:

  • 72 achievement-focused Go controller tests
  • Permission registration coverage for both new resources
  • 15 Playwright definition/category scenarios
  • 13 Playwright character-administration scenarios

Covered behavior includes:

  • Schema fail-closed handling
  • Graph and reference validation
  • Dependency and category cycles
  • Stable identity rules
  • Runtime version fingerprints
  • UTF-8 byte limits
  • BIGINT precision
  • Orphan recovery
  • MariaDB-safe query generation
  • Item icon lookup DTO/rendering
  • Offline mutation enforcement
  • Optimistic concurrency
  • Reward duplicate-risk handling
  • Selectable retry behavior
  • Processing-lease recovery
  • Stale-response suppression
  • Responsive layout behavior

Verification performed

The following targeted Go verification passes on Go 1.23:

go test -count=1 ./boot ./internal/http/controllers ./internal/permissions

A clean Node 20 production build also completes successfully:

cd frontend
npm ci
npm run build

The two Playwright files contain 28 scenarios and collect successfully. Full browser execution requires a running Spire frontend:

npx playwright test \
  tests/achievement-editor.spec.ts \
  tests/character-achievement-editor.spec.ts

Deployment notes

  1. Back up the content and character databases.
  2. Apply the matching EQEmu achievement migrations.
  3. Confirm Spire can resolve eqemu_content and the character database.
  4. Build and deploy Spire normally.
  5. Restart the Spire process or container.
  6. Grant the required editor permissions.
  7. Verify schema readiness in both workspaces.
  8. Author new content disabled.
  9. Resolve all blocking validation findings.
  10. Enable the definition only after review.
  11. Reload the achievement snapshot using the normal EQEmu server command, currently:
#reload achievements global

Intentional limitations

  • Spire does not create or alter the achievement schema.
  • Spire does not directly reload the EQEmu achievement runtime snapshot.
  • Character repairs are blocked while the character is online.
  • Force completion does not directly deliver rewards.
  • Reward retries mark ledger state retryable; EQEmu performs actual delivery.
  • Reset preserves reward and selection history unless the administrator explicitly accepts regrant risk.
  • Definition deletion preserves durable character history.
  • Component presentation-count rows are preserved after definition deletion.
  • Clones intentionally clear cast restrictions.
  • JavaScript is required for nested graph authoring.
  • Browser tests mock API behavior and do not replace live MariaDB/EQEmu integration testing.

Change size

39 files changed
15,478 insertions
6 deletions

Summary by CodeRabbit

  • New Features

    • Added an Achievement Editor for creating, validating, organizing, cloning, and managing achievement definitions.
    • Added Character Achievements administration for viewing progress, completions, rewards, pending actions, and audit history.
    • Added safe controls for updating progress, completing, resetting, and retrying achievement operations.
    • Added schema diagnostics, authoring guidance, reference lookup, responsive layouts, and navigation entries.
  • Bug Fixes

    • Corrected Dual Wield and Make Poison skill labels.
    • Improved item icons, database relationship discovery, and character-directory queries.
  • Tests

    • Added comprehensive automated coverage for editor and administration workflows, validation, concurrency safeguards, and responsive behavior.

* Add achievement definition and character editors

Add full achievement graph authoring with categories, criteria, rewards, selectable sets, cast restrictions, references, contextual help, and fail-closed schema diagnostics.

Add guarded character achievement inspection and repair operations with split-database support, stable identity enforcement, concurrency controls, audit coverage, and offline safety checks.

Wire World Data and administration navigation, register least-privilege resources, add Go and Playwright regression suites, and enable branch-selected Beta release builds in GitHub Actions.

* Fix achievement editor loading queries

Mark manually hydrated graph relations as non-persistent GORM fields so definition, component, and reward-set reads do not attempt invalid association mapping.

Replace the MariaDB-reserved character alias in character summary queries and cover both failures with MySQL dry-run regression tests.

* Prepare achievement query fix release notes

* Render item icons in achievement lookups

Return the structured items.icon value from bounded achievement lookups and render it through Spire's native small item sprite classes, with a missing-icon fallback.

Add backend contract and Playwright coverage and prepare the v5.6.2 Beta fix notes.

* Fix achievement item lookup layout

* Return Workflow back to pre modifications

---------

Co-authored-by: Trust <trust@bastiongame.com>
@coderabbitai

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

This comment was marked as resolved.

@Valorith

Valorith commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Disposition for the CodeRabbit summary and the independent review nitpicks:

  • Fixed native button semantics with aria-pressed, and updated the Playwright contract.
  • Fixed alias lookup with an own-property check.
  • Added a category-delete in-flight guard and disabled modal actions.
  • Strengthened DTO tests to assert GORM discovers no relationships.
  • Counted audit-reason limits in Unicode code points, with regression coverage.
  • Retained the full category hierarchy lock: category mutations already run under the global authoring lock, and the complete locked snapshot keeps parent existence and cycle validation stable.
  • Retained transactional component/criterion replacement: criterion row IDs are not durable editor identity, and whole-graph replacement preserves explicit orphan/recovery and removal semantics.
  • Corrected runtime option snapshots so included enabled options serialize enabled=true rather than carrying the zero value.
  • Suppressed follow-on ownership and wire-width findings after an invalid reward ID.
  • Reused the shared delete-confirmation helper.
  • Derived max_* metadata aliases from the concise limit keys.
  • Cached immutable metadata with sync.Once.
  • Bounded reward-set, reward, component-count, and component validation scans to submitted identities plus the current definition in 34ee7d5; global hierarchy and dependency scans remain intact.
  • Retained best-effort audit cleanup: discardOperationalEditorAudit intentionally returns no error and is shared across operational editors, so cleanup cannot mask the original mutation failure.
  • Passed the active content DB handle on the mutation-discard path.
  • Retained native safety checkboxes after verifying the richer label/help layout and test selectors; replacing them with eq-checkbox would not preserve that contract without unrelated component work.
  • Removed the dead orphan-ID sort.

Validation: controller tests pass, production frontend build passes, and both affected Playwright suites pass 28/28.


Codex

Align achievement definition and character state management with EQEmu schema updates 9329 and 9330. Migrate to provider-neutral reward sources, preserve shared catalog identities, support version zero and pending state updates, and refresh validation, safety controls, help, and regression coverage.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/app/achievements.ts (1)

718-722: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Exclude inactive selectable mappings from the runtime snapshot.

Line 719 includes mappings for enabled rewards even when source_enabled, the reward set, or the mapped option is disabled. These mappings are inactive runtime state.

Gate mapped_rewards with the same source, set, and enabled-option checks used for reward_set. This prevents inactive editor changes from changing the runtime policy snapshot.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/app/achievements.ts` around lines 718 - 722, Update the
mappedRewards construction to retain only mappings whose source, reward set, and
mapped option are enabled, reusing the same eligibility checks used for
reward_set. Apply these checks before filtering enabled reward tokens so
inactive editor mappings cannot enter the runtime snapshot, while preserving the
existing canonical ID mapping and runtimeSort behavior.
🧹 Nitpick comments (2)
internal/http/controllers/achievement_editor_schema_test.go (1)

91-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert index definitions, not only index counts.

These checks pass if an index uses incorrect columns or loses its unique constraint. Assert the column lists and uniqueness for reward_option_entries, reward_source_entries, and character_achievement_pending_updates.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/http/controllers/achievement_editor_schema_test.go` around lines 91
- 103, The test TestAchievementEditorSchemaSpecMatchesFinalSourceIndexes
currently verifies only index counts; update it to inspect each index definition
for reward_option_entries, reward_source_entries, and
character_achievement_pending_updates, asserting the expected column lists and
unique constraints in addition to the counts.
internal/http/controllers/achievement_editor_repository.go (1)

606-618: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle a missing global presentation-count row for a shared component.

Take(&stored) returns gorm.ErrRecordNotFound when achievement_associations has no row for component.ComponentID. The save then fails with a bare record-not-found error instead of a field error. loadDefinition shows that count rows can be absent for existing component IDs, so this path is reachable.

♻️ Proposed handling
-		if err := tx.Table("achievement_associations").Clauses(clause.Locking{Strength: "UPDATE"}).
-			Where("component_id = ?", component.ComponentID).Take(&stored).Error; err != nil {
-			return err
-		}
-		if stored.RequiredCount != component.PresentationCount {
+		countResult := tx.Table("achievement_associations").Clauses(clause.Locking{Strength: "UPDATE"}).
+			Where("component_id = ?", component.ComponentID).Take(&stored)
+		if countResult.Error != nil && !errors.Is(countResult.Error, gorm.ErrRecordNotFound) {
+			return countResult.Error
+		}
+		if countResult.Error == nil && stored.RequiredCount != component.PresentationCount {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/http/controllers/achievement_editor_repository.go` around lines 606
- 618, Update the shared-component validation around the local stored struct and
Take call to handle gorm.ErrRecordNotFound as a field validation error for
components.<index>.presentation_count, using the same 422 error style as the
existing count-mismatch branch. Preserve propagation of other database errors
and the current mismatch validation for found rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/app/achievements.ts`:
- Around line 446-453: Preserve the selectable reward mapping order in the
normalization block by returning mapping.sequence directly instead of the linked
reward’s global sequence. In the mapped_rewards snapshot construction near the
mapped reward handling, include each mapping’s sequence so order-only changes
affect the policy snapshot.

In `@internal/http/controllers/achievement_editor_repository.go`:
- Around line 327-335: The achievementEditorRewardSet field SourceCount is
ignored by GORM, so the source_count query alias is never populated. Update its
GORM mapping to be read-only while retaining the source_count column name,
allowing the query in the reward-set lookup to populate it before Shared is
calculated.

---

Outside diff comments:
In `@frontend/src/app/achievements.ts`:
- Around line 718-722: Update the mappedRewards construction to retain only
mappings whose source, reward set, and mapped option are enabled, reusing the
same eligibility checks used for reward_set. Apply these checks before filtering
enabled reward tokens so inactive editor mappings cannot enter the runtime
snapshot, while preserving the existing canonical ID mapping and runtimeSort
behavior.

---

Nitpick comments:
In `@internal/http/controllers/achievement_editor_repository.go`:
- Around line 606-618: Update the shared-component validation around the local
stored struct and Take call to handle gorm.ErrRecordNotFound as a field
validation error for components.<index>.presentation_count, using the same 422
error style as the existing count-mismatch branch. Preserve propagation of other
database errors and the current mismatch validation for found rows.

In `@internal/http/controllers/achievement_editor_schema_test.go`:
- Around line 91-103: The test
TestAchievementEditorSchemaSpecMatchesFinalSourceIndexes currently verifies only
index counts; update it to inspect each index definition for
reward_option_entries, reward_source_entries, and
character_achievement_pending_updates, asserting the expected column lists and
unique constraints in addition to the counts.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b274b18f-fab0-4d88-ab25-a4e02c05f08a

📥 Commits

Reviewing files that changed from the base of the PR and between 251f4ee and e0e4fba.

📒 Files selected for processing (23)
  • frontend/src/app/achievements.ts
  • frontend/src/assets/css/achievement-editor.css
  • frontend/src/views/achievements/AchievementEditor.vue
  • frontend/src/views/admin/character-achievements/CharacterAchievementEditor.vue
  • internal/http/controllers/achievement_editor_concurrency.go
  • internal/http/controllers/achievement_editor_concurrency_test.go
  • internal/http/controllers/achievement_editor_controller.go
  • internal/http/controllers/achievement_editor_http.go
  • internal/http/controllers/achievement_editor_metadata.go
  • internal/http/controllers/achievement_editor_mutations.go
  • internal/http/controllers/achievement_editor_repository.go
  • internal/http/controllers/achievement_editor_schema.go
  • internal/http/controllers/achievement_editor_schema_test.go
  • internal/http/controllers/achievement_editor_types.go
  • internal/http/controllers/achievement_editor_validation.go
  • internal/http/controllers/achievement_editor_validation_test.go
  • internal/http/controllers/character_achievement_editor_http.go
  • internal/http/controllers/character_achievement_editor_mutations.go
  • internal/http/controllers/character_achievement_editor_mutations_test.go
  • internal/http/controllers/character_achievement_editor_service.go
  • internal/http/controllers/character_achievement_editor_service_test.go
  • tests/achievement-editor.spec.ts
  • tests/character-achievement-editor.spec.ts
🚧 Files skipped from review as they are similar to previous changes (17)
  • internal/http/controllers/character_achievement_editor_http.go
  • internal/http/controllers/achievement_editor_metadata.go
  • frontend/src/assets/css/achievement-editor.css
  • internal/http/controllers/achievement_editor_schema.go
  • internal/http/controllers/achievement_editor_http.go
  • internal/http/controllers/achievement_editor_types.go
  • internal/http/controllers/achievement_editor_controller.go
  • internal/http/controllers/achievement_editor_concurrency_test.go
  • internal/http/controllers/achievement_editor_concurrency.go
  • internal/http/controllers/character_achievement_editor_service.go
  • frontend/src/views/achievements/AchievementEditor.vue
  • internal/http/controllers/character_achievement_editor_mutations_test.go
  • internal/http/controllers/achievement_editor_validation_test.go
  • tests/achievement-editor.spec.ts
  • internal/http/controllers/achievement_editor_mutations.go
  • internal/http/controllers/achievement_editor_validation.go
  • internal/http/controllers/character_achievement_editor_mutations.go

Comment on lines +446 to +453
sequence: (() => {
const token = stringValue(mapping.reward_id)
const reward = token.charAt(0) === '@'
? (source.rewards || [])[Number(token.slice(1))]
: (source.rewards || []).find((row: any) => stringValue(row.reward_id) === token)
return numberValue(reward && reward.sequence, numberValue(mapping.sequence))
})(),
reward_id: stringValue(mapping.reward_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve selectable reward mapping order.

Line 446 replaces reward_option_entries.sequence with the linked reward's global sequence. These fields have different scopes. Saving an unchanged graph can rewrite the order of grants within an option.

Line 718 also omits sequence from mapped_rewards. An order-only runtime change then does not change the policy snapshot.

Keep mapping.sequence during normalization. Include it in each snapshot mapping.

Proposed fix
-      sequence: (() => {
-        const token = stringValue(mapping.reward_id)
-        const reward = token.charAt(0) === '@'
-          ? (source.rewards || [])[Number(token.slice(1))]
-          : (source.rewards || []).find((row: any) => stringValue(row.reward_id) === token)
-        return numberValue(reward && reward.sequence, numberValue(mapping.sequence))
-      })(),
+      sequence: numberValue(mapping.sequence),
       reward_id: stringValue(mapping.reward_id)
     ? graph.reward_set.mappings.filter((mapping: any) => enabledRewardTokens.has(String(mapping.reward_id))).map((mapping: any) => ({
       option_id: mapping.option_id,
+      sequence: mapping.sequence,
       reward_id: canonicalRewardIDs.get(String(mapping.reward_id))
     })).sort(runtimeSort)

Also applies to: 718-722

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/app/achievements.ts` around lines 446 - 453, Preserve the
selectable reward mapping order in the normalization block by returning
mapping.sequence directly instead of the linked reward’s global sequence. In the
mapped_rewards snapshot construction near the mapped reward handling, include
each mapping’s sequence so order-only changes affect the policy snapshot.

Comment on lines +327 to +335
set := achievementEditorRewardSet{Options: make([]achievementEditorRewardOption, 0), Mappings: make([]achievementEditorRewardMapping, 0)}
setResult := r.db.Table("reward_sources source").
Select(`reward_set.reward_set_id, reward_set.title, reward_set.enabled,
source.enabled AS source_enabled,
(SELECT COUNT(*) FROM reward_sources usage_source WHERE usage_source.reward_set_id = source.reward_set_id) AS source_count`).
Joins("JOIN reward_sets reward_set ON reward_set.reward_set_id = source.reward_set_id").
Where("source.source_type = ? AND source.source_id = ?", achievementEditorRewardSourceType, id).Take(&set)
if setResult.Error == nil {
set.Shared = set.SourceCount > 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

GORM struct tag "-" ignore field read and write, "->" read-only permission tag

💡 Result:

In GORM, you can control field-level read and write permissions using the gorm struct tag [1][2]. To create a read-only field (allowing database reads but preventing GORM from writing/updating that field), you should use the -> permission tag [1][3]. For precise control, you can combine the -> (read) and <- (write) operators within the gorm tag [1][4]. Read-only Configuration: To make a field read-only, use: type User struct { Name string gorm:"->" // Read-only: GORM will read but never write/update this field } Permission Tag Reference: - ->: Allows read access [1]. - ->:false: Explicitly disables read access [1][5]. - <-:false: Disables write access (create/update) for the field [1][6]. - gorm:"-": Completely ignores the field (no read, no write, no migration) [1][2]. - gorm:"-:migration": Includes the field in standard CRUD operations but ignores it during AutoMigrate [1][7]. If you want a field to be read-only while still being created in the database schema, use gorm:"->;<-:false" [1][8]. Note that if you use gorm:"-", GORM completely ignores the field for all database operations, meaning it will not be populated by queries and will not be created in the database [1][9][3].

Citations:


🏁 Script executed:

rg -n -C 8 'type achievementEditorRewardSet|SourceCount|source_count|Shared' internal/http/controllers/achievement_editor_repository.go go.mod go.sum

Repository: Valorith/spire

Length of output: 11272


🏁 Script executed:

rg -n -S -C 10 'achievementEditorRewardSet|gorm:"-"' --glob '*.go' .

Repository: Valorith/spire

Length of output: 50370


🏁 Script executed:

files=$(rg -l --glob '*.go' 'type achievementEditorRewardSet')
printf '%s\n' "$files"
for f in $files; do
  rg -n -A 25 -B 5 'type achievementEditorRewardSet' "$f"
done
printf '\nGORM dependencies:\n'
rg -n 'gorm.io/gorm|gorm.io/driver' go.mod go.sum

Repository: Valorith/spire

Length of output: 152


🏁 Script executed:

rg -n --hidden --glob '!vendor/**' --glob '!node_modules/**' 'achievementEditorRewardSet|SourceCount' .
git ls-files | rg 'achievement_editor|go.mod|go.sum'

Repository: Valorith/spire

Length of output: 5770


🏁 Script executed:

sed -n '88,116p' internal/http/controllers/achievement_editor_types.go
printf '\nGORM versions:\n'
rg -n 'gorm.io/gorm|gorm.io/driver' go.mod go.sum | head -20
printf '\nExisting hydration tests:\n'
sed -n '1,70p' internal/http/controllers/achievement_editor_repository_test.go

Repository: Valorith/spire

Length of output: 4765


🏁 Script executed:

mod=$(go env GOPATH 2>/dev/null)/pkg/mod/gorm.io/gorm@v1.24.0
if [ -d "$mod" ]; then
  printf 'GORM source: %s\n' "$mod"
  rg -n -C 6 'TagSettings.*-|\["-"|Permission|ParseField|SchemaField' "$mod/schema" | head -160
else
  printf 'GORM source is not available at %s\n' "$mod"
fi

Repository: Valorith/spire

Length of output: 15820


🏁 Script executed:

mod=$(go env GOPATH)/pkg/mod/gorm.io/gorm@v1.24.0
rg -n -C 8 'Readable|FieldsByDBName|ScanInitialized|ColumnMapping' "$mod/callbacks" "$mod/scan.go" "$mod/schema" | head -220

Repository: Valorith/spire

Length of output: 21686


Make SourceCount read-only instead of ignored

gorm:"-" prevents GORM from scanning the source_count alias, so set.Shared remains false for shared reward sets. Use gorm:"->;column:source_count" or scan the count separately.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/http/controllers/achievement_editor_repository.go` around lines 327
- 335, The achievementEditorRewardSet field SourceCount is ignored by GORM, so
the source_count query alias is never populated. Update its GORM mapping to be
read-only while retaining the source_count column name, allowing the query in
the reward-set lookup to populate it before Shared is calculated.

Add type 6 specific-AA ranks and type 7 class-ineligible fallback authoring, validation, lookup, and character tooling. Bound AA rank-chain validation and add regression coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants