[DRAFT] Вайоленс, наконец-то (завтра) #182
Conversation
Strongest matches foundation infrastructure source Strong matches authority backbone basis core essence essential evidence fundamental groundwork heart key origin principal principle root underpinning Weak matches chief constituent important part primary element
|
крутой код |
Спасибо, я старался |
|
Скажем нет насилию |
|
@coderabbitai review |
Actions performedReview triggered.
|
WalkthroughThe changes introduce and expand functionalities for the Violence gamemode, implementing robust team management, score tracking, and player spawning mechanics. This includes new and modified methods for point adjustments, team assignments, and spawning processes across various system and component files. Additionally, new components and commands empower admin control and player interactions within this gamemode. These enhancements collectively enable a cohesive and dynamic handling of violent game rounds, team dynamics, and player experiences. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin
participant Console as Console Shell
participant VRS as ViolenceRuleSystem
participant PS as PointSystem
participant SPC as SpawnPointComponent
participant SS as StationSpawningSystem
participant PM as PointManagerComponent
Admin->>Console: Execute SwitchTeamCommand
Console->>VRS: Validate and invoke team switch
VRS-->>PS: Update team points
VRS-->>SPC: Update spawn points if needed
note over PS, SPC: Team assignments and spawns are synchronized.
Admin->>VRS: Start Violence Round
VRS->>SS: Initiate player spawning
SS->>SPC: Check spawn points
SPC-->>SS: Return valid spawn points
SS->>VRS: Players spawned
VRS->>PM: Initialize team points
note over PS, PM: Team points and scoreboards are managed.
VRS->>VRS: Handle round progression and scoring
Admin->>VRS: End Violence Round
VRS->>PM: Finalize team points and generate scoreboard
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Outside diff range and nitpick comments (4)
Content.Server/Polymorph/Systems/PolymorphSystem.Map.cs (1)
27-29: The methodEnsurePausedMaphas been correctly made public to support external interactions. Consider adding usage documentation or access restrictions to prevent misuse.Resources/Prototypes/_Miracle/Violence/prototypes.yml (1)
41-41: TODO: Complete the sprite configurations for spawn points.Consider finalizing the sprite states to match the thematic elements of the Violence game mode.
Also applies to: 65-65
Content.Server/Station/Systems/StationSpawningSystem.cs (1)
Line range hint
70-341: Adapt player spawning logic to support team assignments in the new game mode.The modifications to support team-based spawning are a good start. Consider adding more robust error handling and validation for the
teamparameter to ensure it aligns with expected team configurations and constraints within the game mode. This will help prevent issues during gameplay where invalid team data could cause unexpected behavior.Resources/Maps/_Miracle/violenceTestMap.yml (1)
14-21: Check the naming convention forname: map 1337. Ensure it aligns with the project's standards for map naming.Consider using a more descriptive name than "map 1337" for better clarity and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- Content.Server/Points/PointSystem.cs (2 hunks)
- Content.Server/Polymorph/Systems/PolymorphSystem.Map.cs (1 hunks)
- Content.Server/Spawners/Components/SpawnPointComponent.cs (1 hunks)
- Content.Server/Spawners/EntitySystems/SpawnPointSystem.cs (1 hunks)
- Content.Server/Station/Systems/StationSpawningSystem.cs (2 hunks)
- Content.Server/_Miracle/Components/ViolenceParticipatorComponent.cs (1 hunks)
- Content.Server/_Miracle/GameRules/Violence/SwitchTeamCommand.cs (1 hunks)
- Content.Server/_Miracle/GameRules/Violence/ViolenceRuleComponent.cs (1 hunks)
- Content.Server/_Miracle/GameRules/Violence/ViolenceRuleSystem.cs (1 hunks)
- Content.Shared/Points/PointManagerComponent.cs (1 hunks)
- Content.Shared/Points/SharedPointSystem.cs (2 hunks)
- Resources/Maps/_Miracle/violenceTestMap.yml (1 hunks)
- Resources/Prototypes/_Miracle/Violence/game_presets.yml (1 hunks)
- Resources/Prototypes/_Miracle/Violence/prototypes.yml (1 hunks)
- Resources/Prototypes/_Miracle/Violence/testmap.yml (1 hunks)
Files not summarized due to errors (1)
- Resources/Maps/_Miracle/violenceTestMap.yml: Error: Message exceeds token limit
Files skipped from review due to trivial changes (1)
- Resources/Prototypes/_Miracle/Violence/game_presets.yml
Additional context used
yamllint
Resources/Prototypes/_Miracle/Violence/testmap.yml
[error] 17-17: trailing spaces (trailing-spaces)
[warning] 21-21: wrong indentation: expected 4 but found 2 (indentation)
Additional comments not posted (24)
Content.Server/_Miracle/Components/ViolenceParticipatorComponent.cs (1)
7-13: Well-documented and correctly implemented component for match-specific data handling in the Violence game mode.Content.Server/Spawners/Components/SpawnPointComponent.cs (1)
19-24: The addition of theTeamIdproperty is well-documented and correctly implemented, enhancing the functionality of theSpawnPointComponentfor the Violence game mode.Content.Shared/Points/PointManagerComponent.cs (2)
21-25: The addition ofTeamPointsis well-implemented for tracking team scores in the Violence game mode.
34-38: The implementation ofTeamScoreboardfor client-side display of team scores is correctly networked and structured.Content.Server/_Miracle/GameRules/Violence/SwitchTeamCommand.cs (1)
18-44: The implementation ofSwitchTeamCommandis robust, ensuring proper argument handling and team switching logic.Resources/Prototypes/_Miracle/Violence/prototypes.yml (1)
1-91: The new game entities and configurations are well-defined, supporting the structure of the Violence game mode.Content.Server/Spawners/EntitySystems/SpawnPointSystem.cs (1)
37-38: The modifications to support team-specific spawning in the Violence game mode are correctly implemented.Also applies to: 45-46
Content.Shared/Points/SharedPointSystem.cs (1)
114-121: LGTM! The newTeamPointChangedEventis appropriately structured for broadcasting team point changes.Content.Server/_Miracle/GameRules/Violence/ViolenceRuleSystem.cs (13)
41-41: The classViolenceRuleSysteminherits fromGameRuleSystem<ViolenceRuleComponent>and manages the game logic for the Violence game mode.
43-60: All dependencies are properly injected using the[Dependency]attribute. This ensures that the necessary systems and managers are available for the game rule logic.
67-81: Initialization method is well-structured. It subscribes to various local events that are crucial for the game rule logic, such as player spawn and kill events.
83-108: TheActiveTickmethod handles the game rule logic on each tick based on the current round state. This includes ending the round when the time expires and starting the round when its scheduled start time is reached.
158-194: TheOnSpawnCompletemethod ensures that players have the necessary components after spawning. It also handles re-equipping players based on saved equipment, which is important for game continuity.
196-221: TheOnMobStateChangedmethod checks if a player's mob has died and performs necessary updates such as adjusting team counts and potentially ending the round. The logic appears sound, but ensure that all edge cases are handled, such as simultaneous deaths.
223-303: TheStartRoundmethod is responsible for setting up a new round. It includes map management, player spawning, and setting the round state. The method is complex and could benefit from further breakdown into smaller helper methods for clarity and maintainability.
[REFACTOR_SUGGESTion]
369-387: TheEndRoundmethod correctly resets the round state and cleans up the map. It's concise and effective.
389-480: TheOnKillReportedmethod handles kill events, updating scores and potentially triggering round-end conditions. It's a critical part of the game logic that seems to handle various scenarios effectively.
482-499: TheOnPointChangedmethod handles changes in team points and checks if these changes should trigger the end of the round. The logic is straightforward and correctly implemented.
549-560: TheActiveMatchesmethod counts the number of active matches. It's a utility method that could be used in various parts of the system to check game state.
562-603: TheJoinRoundmethod handles player requests to join a round. It checks for eligibility and assigns the player to a team. This method is crucial for managing player participation in the game mode.
605-612: TheLeaveRoundmethod allows players to leave the round. It's straightforward and correctly handles the removal of players from their teams.Resources/Maps/_Miracle/violenceTestMap.yml (3)
1-4: Ensure theformatversion andpostmapinitsettings are compatible with the current game engine version.
66-67: Verify the file path forgravityShakeSoundis correct and the sound file exists at the specified location.Verification successful
The file
alert.oggdoes exist in the repository underResources/Audio/Effects/alert.ogg. This confirms that the path specified in theviolenceTestMap.ymlis correct.
- File path verified:
Resources/Audio/Effects/alert.oggScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the specified sound file exists. # Test: Expect the file to exist. test -f /Audio/Effects/alert.ogg && echo "File exists" || echo "File does not exist"Length of output: 83
Script:
#!/bin/bash # Search for the file 'alert.ogg' in the repository to determine its correct path fd -H 'alert.ogg'Length of output: 120
28-29: Theposattribute in theTransformcomponent should be verified for correctness. Ensure the coordinates are within valid bounds for the map.
| - ViolenceJob | ||
| availableJobs: | ||
| ViolenceJob: [ -1, -1 ] | ||
|
|
There was a problem hiding this comment.
Remove trailing spaces to adhere to YAML syntax best practices.
- ViolenceJob: [ -1, -1 ]
+ ViolenceJob: [ -1, -1]Committable suggestion was skipped due to low confidence.
Tools
yamllint
[error] 17-17: trailing spaces (trailing-spaces)
| - type: entity | ||
| id: ViolenceStation | ||
| parent: | ||
| - BaseStation |
There was a problem hiding this comment.
Correct the indentation for better YAML structure and readability.
- - BaseStation
- - BaseStationJobsSpawning
- - BaseStationRecords
- - BaseStationNanotrasen
+ - BaseStation
+ - BaseStationJobsSpawning
+ - BaseStationRecords
+ - BaseStationNanotrasenCommittable suggestion was skipped due to low confidence.
Tools
yamllint
[warning] 21-21: wrong indentation: expected 4 but found 2 (indentation)
| anchored: False | ||
| pos: 3.5,0.5 | ||
| parent: 2 |
There was a problem hiding this comment.
The anchored attribute for the AlwaysPoweredWallLight should be set to True if the light is not supposed to move.
- anchored: False
+ anchored: TrueCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| anchored: False | |
| pos: 3.5,0.5 | |
| parent: 2 | |
| anchored: True | |
| pos: 3.5,0.5 | |
| parent: 2 |
|
|
||
| // WD EDIT START. Violence gamemode team points | ||
|
|
||
| /// <summary> | ||
| /// Adds the specified point value to a player. | ||
| /// </summary> | ||
| [PublicAPI] | ||
| public void AdjustTeamPointValue(ushort team, FixedPoint2 value, EntityUid uid, PointManagerComponent? component) | ||
| { | ||
| if (!Resolve(uid, ref component)) | ||
| return; | ||
|
|
||
| if (!component.TeamPoints.TryGetValue(team, out var current)) | ||
| current = FixedPoint2.Zero; | ||
|
|
||
| SetTeamPointValue(team, current + value, uid, component); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sets the amount of points for a player | ||
| /// </summary> | ||
| [PublicAPI] | ||
| public void SetTeamPointValue(ushort team, FixedPoint2 value, EntityUid uid, PointManagerComponent? component) | ||
| { | ||
| if (!Resolve(uid, ref component)) | ||
| return; | ||
|
|
||
| if (component.TeamPoints.TryGetValue(team, out var current) && current == value) | ||
| return; | ||
|
|
||
| component.TeamPoints[team] = value; | ||
| component.TeamScoreboard = GetTeamScoreboard(uid, component); | ||
| Dirty(uid, component); | ||
|
|
||
| var ev = new TeamPointChangedEvent(team, value); | ||
| RaiseLocalEvent(uid, ref ev, true); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the amount of points for a given player | ||
| /// </summary> | ||
| [PublicAPI] | ||
| public FixedPoint2 GetTeamPointValue(ushort team, EntityUid uid, PointManagerComponent? component) | ||
| { | ||
| if (!Resolve(uid, ref component)) | ||
| return FixedPoint2.Zero; | ||
|
|
||
| return component.TeamPoints.TryGetValue(team, out var value) | ||
| ? value | ||
| : FixedPoint2.Zero; | ||
| } | ||
|
|
There was a problem hiding this comment.
Refactor to avoid code duplication in team point management methods.
The methods AdjustTeamPointValue, SetTeamPointValue, and GetTeamPointValue share similar logic with their player counterparts. Consider abstracting common functionalities into helper methods to reduce duplication and improve maintainability.
There was a problem hiding this comment.
@rhailrake, the counterparts to the new team point management methods (AdjustTeamPointValue, SetTeamPointValue, GetTeamPointValue) in the PointSystem class are the methods that manage individual player points. These include:
AdjustPointValue: Adds a specified point value to a player's total points.SetPointValue: Sets the total point value for a player.GetPointValue: Retrieves the total point value for a player.
These methods are similar in functionality to the new team methods but operate on individual player points rather than team points. The logic for adjusting, setting, and getting points is quite similar between these sets of methods, which is why refactoring to reduce code duplication was suggested.
There was a problem hiding this comment.
Suggest your version then
There was a problem hiding this comment.
@rhailrake, here's a suggested refactor for the PointSystem class to reduce code duplication between the team and individual player point management methods. I've introduced helper methods to encapsulate common functionalities:
public sealed class PointSystem : SharedPointSystem
{
// Helper methods to abstract common functionalities
private bool TryResolveComponents(EntityUid uid, out PointManagerComponent? component, EntityUid? user = null, ActorComponent? actor = null)
{
return Resolve(uid, ref component) && (user == null || Resolve(user.Value, ref actor, false));
}
private void UpdatePoints<T>(Dictionary<T, FixedPoint2> points, T key, FixedPoint2 value, EntityUid uid, PointManagerComponent? component)
{
if (!points.TryGetValue(key, out var current) || current != value)
{
points[key] = value;
Dirty(uid, component);
}
}
private void RaisePointChangeEvent<T>(T key, FixedPoint2 value, EntityUid uid)
{
var ev = new PointChangedEvent<T>(key, value);
RaiseLocalEvent(uid, ref ev, true);
}
// Refactored team point management methods
public void AdjustTeamPointValue(ushort team, FixedPoint2 value, EntityUid uid, PointManagerComponent? component)
{
if (!TryResolveComponents(uid, out component)) return;
UpdatePoints(component.TeamPoints, team, component.TeamPoints.TryGetValue(team, out var current) ? current + value : value, uid, component);
RaisePointChangeEvent(team, component.TeamPoints[team], uid);
}
public void SetTeamPointValue(ushort team, FixedPoint2 value, EntityUid uid, PointManagerComponent? component)
{
if (!TryResolveComponents(uid, out component)) return;
UpdatePoints(component.TeamPoints, team, value, uid, component);
RaisePointChangeEvent(team, value, uid);
}
public FixedPoint2 GetTeamPointValue(ushort team, EntityUid uid, PointManagerComponent? component)
{
return TryResolveComponents(uid, out component) && component.TeamPoints.TryGetValue(team, out var value) ? value : FixedPoint2.Zero;
}
// Example of refactoring individual point methods similarly
public void AdjustPointValue(EntityUid user, FixedPoint2 value, EntityUid uid, PointManagerComponent? component, ActorComponent? actor = null)
{
if (!TryResolveComponents(uid, out component, user, actor)) return;
var userId = actor.PlayerSession.UserId;
UpdatePoints(component.Points, userId, component.Points.TryGetValue(userId, out var current) ? current + value : value, uid, component);
RaisePointChangeEvent(userId, component.Points[userId], uid);
}
}This refactoring introduces three helper methods: TryResolveComponents, UpdatePoints, and RaisePointChangeEvent, which are used in both the team and individual point management methods to handle common operations such as component resolution, point updating, and event raising. This reduces duplication and enhances maintainability.
| /// <summary> | ||
| /// This method handles roundstart player spawning. After roundstart, players will be spawned by RespawnRuleSystem. | ||
| /// </summary> | ||
| /// <param name="ev"></param> | ||
| private void OnBeforeSpawn(PlayerBeforeSpawnEvent ev) | ||
| { | ||
| /* | ||
| if (ev.LateJoin) // this will allow this gamerule to be added to usual rounds. maybe | ||
| return; | ||
| */ | ||
| var query = EntityQueryEnumerator<ViolenceRuleComponent, RespawnTrackerComponent, PointManagerComponent, GameRuleComponent>(); | ||
| while (query.MoveNext(out var uid, out var ruleComponent, out var tracker, out var point, out var rule)) | ||
| { | ||
| if (!GameTicker.IsGameRuleActive(uid, rule)) | ||
| continue; | ||
|
|
||
| var teamList = TeamsEligibleToJoin(ruleComponent); | ||
| if (teamList.Count == 0) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| if (!ruleComponent.TeamMembers.TryGetValue(ev.Player.UserId, out var team)) | ||
| { | ||
| ruleComponent.TeamMembers.Add(ev.Player.UserId, teamList[_robustRandom.Next(0, teamList.Count())]); | ||
| } | ||
|
|
||
| var newMind = _mind.CreateMind(ev.Player.UserId, ev.Profile.Name); | ||
| _mind.SetUserId(newMind, ev.Player.UserId); | ||
|
|
||
| var mobMaybe = _stationSpawning.SpawnPlayerCharacterOnStation(ev.Station, null, ev.Profile, null, team); | ||
| DebugTools.AssertNotNull(mobMaybe); | ||
| var mob = mobMaybe!.Value; | ||
|
|
||
| _mind.TransferTo(newMind, mob); | ||
| // Should get startingGear from the client here. Setting default startingGear if none or invalid is passed. | ||
| // Also different startingGear for different teams. A dictionary of teams and startingGear in ViolenceRuleComponent? | ||
| //SetOutfitCommand.SetOutfit(mob, ruleComponent.Gear, EntityManager); | ||
| EnsureComp<KillTrackerComponent>(mob); | ||
| _respawn.AddToTracker(ev.Player.UserId, uid, tracker); | ||
|
|
||
| _point.EnsurePlayer(ev.Player.UserId, uid, point); | ||
|
|
||
| ev.Handled = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
The method OnBeforeSpawn handles player spawning at the beginning of the round. It assigns players to teams and ensures that they are properly equipped and tracked. Consider adding more comments to explain the logic, especially how teams are determined and players are equipped.
|
|
||
| public bool DoRoundEndBehavior(ViolenceRuleComponent comp) | ||
| { | ||
| // announce the winner | ||
| foreach (var (player, team) in comp.TeamMembers) | ||
| { | ||
| if (team != comp.MatchVictor) | ||
| continue; | ||
|
|
||
| if (!_playerManager.SessionsDict.TryGetValue(player, out var session)) | ||
| continue; | ||
|
|
||
|
|
||
| _polymorphSystem.EnsurePausedMap(); | ||
|
|
||
| if (session.AttachedEntity != null && | ||
| TryComp<MobStateComponent>(session.AttachedEntity, out var mobState) && | ||
| mobState.CurrentState == MobState.Alive) | ||
| { | ||
| if (_polymorphSystem.PausedMap != null) | ||
| { | ||
| if (!comp.SavedEquip.ContainsKey(session.UserId)) | ||
| { | ||
| comp.SavedEquip.Add(session.UserId, new List<EntityUid>()); | ||
| } | ||
|
|
||
| if (TryComp<HandsComponent>(session.AttachedEntity, out var hands)) | ||
| { | ||
| foreach (var hand in _hands.EnumerateHeld(session.AttachedEntity.Value)) | ||
| { | ||
| _hands.TryDrop(session.AttachedEntity.Value, hand, checkActionBlocker: false); | ||
| _transform.SetParent(hand, Transform(hand), _polymorphSystem.PausedMap.Value); | ||
| comp.SavedEquip[session.UserId].Add(hand); | ||
| comp.EquipSlots.Add(hand, "hand"); | ||
| } | ||
| } | ||
| if (TryComp<InventoryComponent>(session.AttachedEntity, out var inv)) | ||
| { | ||
| var enumerator = new InventorySystem.InventorySlotEnumerator(inv); | ||
| while (enumerator.NextItem(out var item, out var slot)) | ||
| { | ||
| _inventory.TryUnequip(session.AttachedEntity.Value, slot.Name, true, true); | ||
| _transform.SetParent(item, Transform(item), _polymorphSystem.PausedMap.Value); | ||
| comp.SavedEquip[session.UserId].Add(item); | ||
| comp.EquipSlots.Add(item, slot.Name); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ummm TODO: test this | ||
| if (comp.Money.ContainsKey(session.UserId)) | ||
| { | ||
| comp.Money[session.UserId] += comp.AliveReward; | ||
| } | ||
| else | ||
| { | ||
| comp.Money.TryAdd(session.UserId, comp.AliveReward); | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
The DoRoundEndBehavior method handles the end of a round, including announcing the winner and handling player equipment. The method is well-implemented but consider refining the logic to handle player rewards more efficiently.
| /// <summary> | ||
| /// I will rewrite this to not require component input if needed. In fact, TODO | ||
| /// </summary> | ||
| /// <param name="player"></param> | ||
| /// <param name="comp"></param> | ||
| /// <param name="item"></param> | ||
| /// <returns></returns> | ||
| public bool TryBuyItem(NetUserId player, ViolenceRuleComponent comp, string item) | ||
| { | ||
| if (!comp.TeamMembers.TryGetValue(player, out var team)) | ||
| return false; | ||
|
|
||
| if (!comp.Shop.TryGetValue(team, out var shop)) | ||
| return false; | ||
|
|
||
| if (!shop.TryGetValue(item, out var listing)) | ||
| return false; | ||
|
|
||
| if (!comp.Money.TryGetValue(player, out var money)) | ||
| return false; | ||
|
|
||
|
|
||
| if (money >= listing.Price) | ||
| { | ||
| if (!_prototypeManager.TryIndex(item, out var proto)) | ||
| return false; | ||
|
|
||
| comp.Money[player] -= listing.Price; | ||
|
|
||
| if (!_playerManager.GetSessionById(player).AttachedEntity.HasValue) | ||
| { | ||
|
|
||
| } | ||
|
|
||
| _polymorphSystem.EnsurePausedMap(); | ||
| var equip = EntityManager.SpawnEntity(item, new EntityCoordinates(_polymorphSystem.PausedMap!.Value, 0, 0)); | ||
|
|
||
| if (!comp.SavedEquip.ContainsKey(player)) | ||
| { | ||
| comp.SavedEquip.Add(player, new List<EntityUid>()); | ||
| } | ||
| comp.SavedEquip[player].Add(equip); | ||
| comp.EquipSlots.Add(equip, listing.Slot); | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
The TryBuyItem method allows players to buy items using in-game currency. The method checks several conditions to ensure the transaction is valid. Consider adding error logging for failed transactions to aid in debugging.
| using Robust.Shared.Console; | ||
| using Robust.Shared.Network; | ||
|
|
||
| // TODO: test permissions |
There was a problem hiding this comment.
TODO: Ensure proper permissions are tested for the SwitchTeamCommand.
Would you like me to help implement the permission checks or create a test case for this command?
7 participants
Описание PR
Скриншоты
Чек-лист:
typo:
Изменения
Changelog
🆑
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation