-
-
Notifications
You must be signed in to change notification settings - Fork 311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tidemind event #1624
tidemind event #1624
Conversation
## Walkthrough
Hark, noble coder! In this latest enchantment upon our server realm, a new class named `JobAddComponentsRule` has been conjured. It dwells within the station events and diligently iterates over entities bearing a `MindContainerComponent` to ascertain if their mind's job aligns with one of the affected prototypes. If so, the rule bestows additional components upon the entity, with an option to purge any existing ones. Should a message be inscribed within the component’s scroll, a localized chat missive is sent forth to the corresponding session. Additionally, a fresh game rule known as `TidemindDevelop` emerges, which combines the station event and job component magic. This spell also introduces a new radio channel, `TideHivemind`, and augments the localization tomes with suitable messages for the event.
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Rule as JobAddComponentsRule
participant Query as Entity Query (MindContainerComponent)
participant Mind as MindSystem
participant Session as PlayerSession
participant Chat as ChatManager
Rule->>Query: Fetch entities with MindContainerComponent
Query-->>Rule: Return each valid entity
Rule->>Mind: Check if entity's mind holds a matching job
Mind-->>Rule: Confirm job match
Rule->>Query: Add specified components (removing old ones if needed)
alt Message is defined
Rule->>Session: Retrieve player's session from mind
Session-->>Rule: Return session
Rule->>Chat: Dispatch localized chat message
end sequenceDiagram
participant GR as TidemindDevelop Event
participant SE as StationEvent Component
participant JR as JobAddComponentsRule
participant Entity as Passenger Entity
GR->>SE: Trigger TidemindDevelop event
SE->>JR: Invoke JobAddComponentsRule mechanism
JR->>Entity: Verify job and add radio components (IntrinsicRadioReceiver, Transmitter, ActiveRadio)
Possibly related PRs
Suggested reviewers
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRuleComponent.cs (1)
10-24
: HEAR YE! Your magical fields lack the ancient scrolls of documentation!While your enchantments are strong, future wizards may struggle to comprehend their purpose without proper documentation scrolls! Consider adding XML documentation comments for these mystical fields:
+ /// <summary> + /// List of job prototypes that will be affected by this rule. + /// </summary> [DataField(required: true)] public List<ProtoId<JobPrototype>> Affected = default!; + /// <summary> + /// Whether to remove existing components before adding new ones. + /// </summary> [DataField] public bool RemoveExisting = true; + /// <summary> + /// Registry of components to be added to the affected entities. + /// </summary> [DataField(required: true)] public ComponentRegistry Components = new();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRule.cs
(1 hunks)Content.Server/_Goobstation/StationEvents/JobAddComponentsRuleComponent.cs
(1 hunks)Resources/Locale/en-US/_Goobstation/headset/headsets.ftl
(1 hunks)Resources/Locale/en-US/_Goobstation/station-events/events/tidemind.ftl
(1 hunks)Resources/Prototypes/GameRules/events.yml
(1 hunks)Resources/Prototypes/_Goobstation/GameRules/events.yml
(1 hunks)Resources/Prototypes/_Goobstation/radio_channels.yml
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Resources/Locale/en-US/_Goobstation/headset/headsets.ftl
- Resources/Locale/en-US/_Goobstation/station-events/events/tidemind.ftl
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: YAML Linter
- GitHub Check: YAML map schema validator
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (6)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRuleComponent.cs (2)
1-6
: BEHOLD! These imports are worthy of my arcane approval!The magical scrolls you have summoned are precisely what this enchantment requires, no more, no less!
7-9
: AHA! A well-warded component you have crafted!The seals of power are strong with this one! The RegisterComponent inscription and Access ward shall protect our magical component from unauthorized tampering!
Content.Server/_Goobstation/StationEvents/JobAddComponentsRule.cs (1)
12-41
: BEHOLD, A MOST EXCELLENT IMPLEMENTATION!The arcane runes of dependency injection and the sacred null checks have been properly inscribed. The mystical iteration through minds is both elegant and powerful!
Resources/Prototypes/_Goobstation/radio_channels.yml (1)
9-15
: VERIFY THE MYSTICAL KEYCODE BINDING!O seeker of knowledge, we must ensure the keycode 'p' does not conflict with other radio channel enchantments.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other radio channels using keycode 'p' rg "keycode: ['|\"]p['|\"]" -A 5Length of output: 322
HEAR YE, MORTAL! THE MYSTICAL KEYCODE 'p' IS SINGULAR IN ITS POTENCY!
After perusing the sacred scrolls of our radio channel repository, only the new TideHivemind channel brandishes the keycode 'p'. No other incantation of that symbol has been unearthed, and thus no dire conflicts lurk in its shadows.
- Unique binding confirmed in Resources/Prototypes/_Goobstation/radio_channels.yml
Resources/Prototypes/_Goobstation/GameRules/events.yml (1)
1-21
: THE CONFIGURATION SCROLLS ARE WELL SCRIBED!The mystical components have been properly bound to the TidemindDevelop enchantment. The weight of 10 and duration of 1 are balanced numbers in the arcane arts!
Resources/Prototypes/GameRules/events.yml (1)
27-27
: THE EVENT HAS BEEN PROPERLY INSCRIBED INTO THE ANCIENT TABLE!The TidemindDevelop enchantment has found its rightful place among the other calm events in the BasicCalmEventsTable. The comment marking it as Goobstation serves as a proper seal of origin!
Now this is more like a feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it temporary.
Content.Server/_Goobstation/StationEvents/JobAddComponentsRuleComponent.cs
Outdated
Show resolved
Hide resolved
too evil, i could just make it rare instead |
It being rare and for the whole round after it's rolled is fine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Resources/Prototypes/_Goobstation/GameRules/events.yml (1)
4-8
: Lo! The StationEvent component is imbued with rare power.
The "StationEvent" component is configured with a weight of 3 (truly rare) and a duration of 1. Note the comment "no occurences limit"—might I suggest correcting "occurences" to "occurrences" for the sake of clarity in our ancient scrolls?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Resources/Prototypes/_Goobstation/GameRules/events.yml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
- GitHub Check: YAML map schema validator
- GitHub Check: YAML Linter
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (5)
Resources/Prototypes/_Goobstation/GameRules/events.yml (5)
1-3
: Hark, noble coder, the entity declaration is most pleasing!
The new entity "TidemindDevelop" inheriting from "BaseGameRule" is declared with proper attributes. Verify that this entity type aligns with the expected schema of thy game engine.
9-13
: Behold, the JobAddComponentsRule component is well-forged.
Thou hast aptly specified a message identifier ("station-event-tidemind-message") and targeted the "Passenger" with the necessary additional components. This configuration shall indeed work its magic upon the affected entities.
14-15
: The IntrinsicRadioReceiver and its kin are summoned with precision.
The "IntrinsicRadioReceiver" component and the "IntrinsicRadioTransmitter" component (to be blessed with the "TideHivemind" channel) are declared in a clear and orderly fashion.
16-17
: The channels for the Transmitter are declared in a manner most clear.
The list of channels correctly includes "TideHivemind". This clear delineation of communication paths ensures that thy entities shall listen to the call of unity.
18-20
: ActiveRadio component's channel configuration is artfully wrought.
The ActiveRadio component, also tied to the "TideHivemind" channel, complements the event perfectly, ensuring that the magic of the tidemind spreads throughout the realm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRule.cs (1)
33-34
: OPTIMIZE THY MESSAGE ENCHANTMENT!O wise one, the message localization could be made more efficient by pre-localizing the component message once before the entity iteration. This would prevent redundant translations for each affected entity.
+ var localizedMessage = component.Message != null + ? Loc.GetString("chat-manager-server-wrap-message", ("message", Loc.GetString(component.Message))) + : null; while (query.MoveNext(out var target, out var mindContainer)) { // ... existing code ... - var message = Loc.GetString("chat-manager-server-wrap-message", ("message", Loc.GetString(component.Message))); - _chat.ChatMessageToOne(ChatChannel.Local, message, message, EntityUid.Invalid, false, session.Channel); + _chat.ChatMessageToOne(ChatChannel.Local, localizedMessage!, localizedMessage!, EntityUid.Invalid, false, session.Channel);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRule.cs
(1 hunks)Resources/Locale/en-US/_Goobstation/station-events/events/tidemind.ftl
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/Locale/en-US/_Goobstation/station-events/events/tidemind.ftl
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: YAML Linter
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Test Packaging
- GitHub Check: YAML map schema validator
🔇 Additional comments (2)
Content.Server/_Goobstation/StationEvents/JobAddComponentsRule.cs (2)
1-17
: BEHOLD! A MOST EXCELLENT SPELL STRUCTURE!The arcane dependencies are properly bound, and the class is sealed from modification by lesser wizards. Your mastery of the mystical arts of dependency injection is commendable!
20-39
: HARK! THY QUERY LOOP REQUIRES FORTIFICATION!Most noble spellcaster, while thy enchantment is powerful, it could be strengthened further:
- The
break
statement at line 36 means only the first matching prototype's components shall be bestowed. Is this thy intended sorcery?- The component addition ritual lacks protection against failed transmutations.
Let us fortify thy spell with this enchanted scroll:
foreach (var proto in component.Affected) { if (_job.MindHasJobWithId(mindContainer.Mind, proto)) { - EntityManager.AddComponents(target, component.Components, component.RemoveExisting); + try + { + EntityManager.AddComponents(target, component.Components, component.RemoveExisting); + } + catch (Exception e) + { + Logger.ErrorS("tidemind", $"Failed to add components to {ToPrettyString(target)}: {e}"); + continue; + } if (component.Message != null && _mind.TryGetSession(mindContainer.Mind.Value, out var session)) { var message = Loc.GetString("chat-manager-server-wrap-message", ("message", Loc.GetString(component.Message))); _chat.ChatMessageToOne(ChatChannel.Local, message, message, EntityUid.Invalid, false, session.Channel); } - break; + // Remove break to allow multiple prototype matches to apply their components } }
About the PR
adds tidemind for all present tiders as a midround event
Why / Balance
funny
also piras said he'd merge it
Technical details
see diff, actually implements a JobAddComponentsRule which can be reused for other stuff
if you're reverting this, please instead only comment out the tidemind event
Media
Changelog
🆑