feat(guildserver): refresh cached guild roster on reads + recover online state - #56
Closed
3kynox wants to merge 7 commits into
Closed
feat(guildserver): refresh cached guild roster on reads + recover online state#563kynox wants to merge 7 commits into
3kynox wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the guildserver to better handle guild roster consistency and online-state accuracy in a clustered environment, while also adding a CreateGuild RPC and corresponding persistence/event plumbing (stacked on #55).
Changes:
- Added
CreateGuildRPC, repository implementation, andguild.createdevent publishing. - Improved in-memory guild cache behavior by supporting periodic re-hydration and tracking online state independently of roster membership.
- Added startup online-state recovery via the characters service (
GetOnlineCharacters) and introduced related configuration.
Reviewed changes
Copilot reviewed 15 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/events/producer-guild.go | Adds GuildCreated event production API. |
| shared/events/mocks/GuildServiceProducer.go | Updates producer mock to include GuildCreated. |
| shared/events/events-guild.go | Introduces guild.created subject + payload type. |
| api/proto/v1/guilds/guilds.proto | Adds CreateGuild RPC and messages. |
| gen/guilds/pb/guilds.pb.go | Regenerated protobuf code to include create guild messages. |
| gen/guilds/pb/guilds_grpc.pb.go | Regenerated gRPC stubs to include CreateGuild. |
| apps/guildserver/server/guilds.go | Implements the CreateGuild RPC handler. |
| apps/guildserver/server/guilds-logger_debug.go | Adds debug logging middleware for CreateGuild. |
| apps/guildserver/service/guilds.go | Adds service-level CreateGuild with validation + event publish. |
| apps/guildserver/service/guilds-create_test.go | Adds unit tests covering guild creation behavior. |
| apps/guildserver/service/guilds-cache.go | Extends cache interface with online seeding + source membership lookup. |
| apps/guildserver/service/guilds-cache_inmem.go | Adds refresh throttling, online overlay, and cache-level CreateGuild. |
| apps/guildserver/service/guilds-cache_inmem_test.go | Adds tests for refresh/online overlay/source membership + cache create behavior. |
| apps/guildserver/service/guilds_test.go | Adjusts tests to account for refresh throttling behavior. |
| apps/guildserver/repo/guilds.go | Adds ErrGuildNameTaken + CreateGuild to repo interface. |
| apps/guildserver/repo/guilds_mysql.go | Implements GuildByRealmAndID and transactional CreateGuild with retry on PK conflict. |
| apps/guildserver/repo/mocks/guilds-repo.go | Updates repo mock with CreateGuild. |
| apps/guildserver/config/config.go | Adds characters service address config for online-state recovery. |
| apps/guildserver/cmd/guildserver/main.go | Seeds online state on startup via characters service. |
Files not reviewed (4)
- apps/guildserver/repo/mocks/guilds-repo.go: Generated file
- gen/guilds/pb/guilds.pb.go: Generated file
- gen/guilds/pb/guilds_grpc.pb.go: Generated file
- shared/events/mocks/GuildServiceProducer.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+508
to
+512
| g.cacheMutex.Lock() | ||
| g.cache[realmID][id] = guild | ||
| for _, member := range guild.GuildMembers { | ||
| if member.PlayerGUID == leaderGUID { | ||
| // Guild creation is always driven by a live session of the leader, |
Contributor
Author
There was a problem hiding this comment.
Added an init guard for the per-realm inner maps in the shared CreateGuild commit.
Comment on lines
126
to
130
|
|
||
| seedOnlineChars(cfg, cache, 1) | ||
|
|
||
| return service.NewGuildService(cache, events.NewGuildServiceProducerNatsJSON(natsCon, guildserver.Ver)) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Kept consistent with the existing single-realm assumption: the cache Warmup on master hardcodes realm 1 the same way. Multi-realm wiring would be a separate change across the service.
3kynox
force-pushed
the
feat/guildserver-roster-refresh-online
branch
4 times, most recently
from
July 22, 2026 17:20
d3829e4 to
d599f27
Compare
Adds a CreateGuild RPC: the guild row, the five default 3.3.5 ranks and the
leader as guild master are inserted in one transaction, the guild id being
allocated as MAX(guildid)+1 with a retry on primary key conflicts (a
worldserver can still create guilds in-process concurrently). The created
guild is cached hydrated and a guild.created event is published.
Also implements the GuildByRealmAndID mysql repo method that was left as
panic("implement me") - the cache needs it to hydrate the new guild.
(cherry picked from commit 3608d5a3f14348c31ca54faafb07f27948919bfc)
…ild creation The world can disband guilds without going through the guild service (e.g. GM .guild delete handled in-process), leaving a stale positive membership in the in-memory cache. CreateGuild now rechecks a positive answer against the database and evicts the stale cache entry, instead of rejecting with 'already in guild' until the service restarts. Also treats a missing guild_member row as 'not in a guild' in the MySQL repo instead of surfacing sql.ErrNoRows. (cherry picked from commit 2c779169db1722cb20bfa58faf8674a482af09f1)
Guild creation is always driven by a live session of the leader, but in cluster mode the world doesn't flush the online flag to the characters table, so the hydration read the leader back as offline. This left the fresh guild with an empty online-members list: the roster showed the leader disconnected and guild events (e.g. MOTD updates) were broadcast to nobody until the leader relogged. (cherry picked from commit 7000ccaa6c9f19ce89c1f3884f355243a74628d9)
…ity) The core's CreateDefaultGuildRanks gives both Guild Master and Officer GR_RIGHT_ALL; the service only gave the Officer chat rights, so a freshly promoted officer had every guild action greyed out client-side (invite, promote, etc.). Match the core layout and assert it in the create test.
- CreateGuildParams gains signatoryGUIDs; members are inserted with the lowest rank in the same transaction, skipping characters that joined another guild since signing. - guild.created event now carries the added member guids so worldservers can populate the state for the new guild. - New worldserver RPC CanTurnInGuildPetition for the gateway to validate petitions on the worldserver side before calling CreateGuild.
The world can mutate guild_member without going through the service (petition turn-in, in-process invites, GM commands), leaving the cache blind to those members until restart. Guild reads now re-hydrate the guild from the repo, throttled to one refresh per guild every 3s. Online statuses are overlaid from a new gateway-events-driven online set that also tracks characters that aren't guild members yet, since characters.online isn't maintained for cluster sessions. (cherry picked from commit 4d9df78)
3kynox
force-pushed
the
feat/guildserver-roster-refresh-online
branch
from
July 23, 2026 07:14
b6471c1 to
6ed40ab
Compare
3kynox
force-pushed
the
feat/guildserver-roster-refresh-online
branch
from
July 23, 2026 07:16
d599f27 to
b6471c1
Compare
…rvice on startup Login events observed before the process started are gone, so members hydrated from the DB stay offline until they relog. On startup the online state is recovered from the characters service (GetOnlineCharacters) and overlaid on the cached rosters, including guilds created in-process. Best effort: disabled when the address is empty, degrades to stale statuses on failure.
3kynox
force-pushed
the
feat/guildserver-roster-refresh-online
branch
from
July 23, 2026 07:41
6ed40ab to
a5b5d0c
Compare
Contributor
Author
|
Superseded by #81, which regroups the guild creation work into a single PR as discussed. Same content, rebased on current master, with the review points from this PR already applied. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #55 — its three commits show here until #55 merges; my changes are the top two commits (roster refresh + online recovery). Please review after #55.
The in-memory guild cache only learned about mutations that went through this service, but the world can change
guild_memberdirectly (petition turn-in, in-process invites, GM commands), so rosters served to clients diverged from the DB until a restart.GuildByRealmAndIDnow re-hydrates from the repo (throttled, 3s/guild), evicts deleted guilds and overlays online statuses tracked from the gateway login/logout events (independently of membership). On startup the online state is recovered from the characters service (GetOnlineCharacters), since events published before boot are gone.