Conversation
WalkthroughThe changes enhance the home decoration feature across the system. New properties and methods in the Home classes now track decoration levels, experience, and reward timestamps. Conversion operators, JSON configuration, and SQL updates have been adjusted accordingly. Additionally, a new MasteryUgcHousingTable has been integrated with lazy-loading, parsing, and JSON type mapping. Command handling and packet serialization have been updated to support interior design check-in and rewards, while new constants and a package update further support these features. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 using PR comments)
Other keywords and placeholders
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: 3
🧹 Nitpick comments (8)
Maple2.Server.Game/Commands/HomeCommand.cs (1)
60-77: Consider consolidating exp-related commands.
SetExpCommandappears to duplicate functionality fromExpCommand. Consider consolidating these into a single command with a flag to determine whether to useGainExpor direct assignment.-private class SetExpCommand : Command { +private class ExpCommand : Command { private readonly GameSession session; + private readonly TableMetadataStorage tableMetadata; - public SetExpCommand(GameSession session) : base("setexp", "Set home decoration exp.") { + public ExpCommand(GameSession session, TableMetadataStorage tableMetadata) : base("exp", "Set home decoration exp.") { this.session = session; + this.tableMetadata = tableMetadata; var exp = new Argument<int>("exp", "Decoration exp."); + var direct = new Option<bool>("--direct", "Set exp directly without using GainExp."); AddArgument(exp); - this.SetHandler<InvocationContext, int>(Handle, exp); + AddOption(direct); + this.SetHandler<InvocationContext, int, bool>(Handle, exp, direct); } - private void Handle(InvocationContext context, int exp) { + private void Handle(InvocationContext context, int exp, bool direct) { Home home = session.Player.Value.Home; - home.DecorationExp = exp; + if (direct) { + home.DecorationExp = exp; + } else { + home.GainExp(exp, tableMetadata.MasteryUgcHousingTable.Entries); + } session.Send(CubePacket.DesignRankReward(home)); } }Maple2.Database/Model/Map/Home.cs (1)
20-23: Consider adding validation for decoration properties.The decoration-related properties should have validation to ensure they don't contain invalid values.
Consider adding validation through property setters:
- public long DecorationLevel { get; set; } - public long DecorationExp { get; set; } - public long DecorationRewardTimestamp { get; set; } + private long decorationLevel; + public long DecorationLevel { + get => decorationLevel; + set => decorationLevel = value >= 0 ? value : throw new ArgumentException("Decoration level cannot be negative."); + } + private long decorationExp; + public long DecorationExp { + get => decorationExp; + set => decorationExp = value >= 0 ? value : throw new ArgumentException("Decoration exp cannot be negative."); + } + private long decorationRewardTimestamp; + public long DecorationRewardTimestamp { + get => decorationRewardTimestamp; + set => decorationRewardTimestamp = value >= 0 ? value : throw new ArgumentException("Decoration reward timestamp cannot be negative."); + }Maple2.Model/Game/User/Home.cs (1)
70-76: Consider making the default values configurable.The method hardcodes several values that might need to change in the future:
- Expiry date set to year 2900
- Default welcome message
- Initial decoration level
- Default passcode pattern
Consider moving these values to configuration:
+public class HomeDefaults { + public required DateTimeOffset ExpiryTime { get; init; } + public required string WelcomeMessage { get; init; } + public required int InitialDecorationLevel { get; init; } + public required string DefaultPasscode { get; init; } +} public void NewHomeDefaults(string characterName) { Indoor.Name = characterName; - Indoor.ExpiryTime = new DateTimeOffset(2900, 12, 31, 0, 0, 0, TimeSpan.Zero).ToUnixTimeSeconds(); - Message = "Thanks for visiting. Come back soon!"; - DecorationLevel = 1; - Passcode = "*****"; + Indoor.ExpiryTime = homeDefaults.ExpiryTime.ToUnixTimeSeconds(); + Message = homeDefaults.WelcomeMessage; + DecorationLevel = homeDefaults.InitialDecorationLevel; + Passcode = homeDefaults.DefaultPasscode; }Maple2.Server.Game/Manager/HousingManager.cs (3)
352-364: Consider making the dictionary readonly.The
decorationGoaldictionary contains constant data that shouldn't change during runtime. Making it readonly would prevent accidental modifications.- private readonly Dictionary<HousingCategory, int> decorationGoal = new() { + private static readonly Dictionary<HousingCategory, int> decorationGoal = new() {
455-476: Add validation for reward ID.The method should validate that the reward ID is within an acceptable range before processing.
public void InteriorReward(byte rewardId) { + if (rewardId >= tableMetadata.MasteryUgcHousingTable.Entries.Count) { + Logger.Error("Invalid reward ID: {RewardId}", rewardId); + return; + } if (Home.InteriorRewardsClaimed.Contains(rewardId)) { return; }
300-301: Add XML documentation for the method.The method would benefit from XML documentation explaining its purpose and parameters.
+ /// <summary> + /// Initializes a new home with default settings. + /// </summary> + /// <param name="characterName">The name of the character owning the home.</param> + /// <param name="template">Optional template for home initialization.</param> public void InitNewHome(string characterName, ExportedUgcMapMetadata? template) { Home.NewHomeDefaults(characterName);Maple2.Model/Metadata/Constants.cs (1)
112-124: Consider grouping related constants.The new constants would be more maintainable if grouped by their purpose (e.g., chat items, herbs, home decoration).
+ #region Chat Items public const int ChannelChatVoucherId = 20300039; public const int WorldChatVouncherId = 20300040; public const int FreeRotorsWalkieTalkieId = 20300078; + #endregion + #region Consumable Items public const int CubeFragmentId = 20300559; public const int ExpBooster100Id = 20000071; public const int AssortedBlockChestId = 20301018; + #endregion + #region Herbs public const int RedHerbId = 20000028; public const int OrangeHerbId = 20000029; public const int WhiteHerbId = 20000030; public const int SpecialRedHerbId = 20000031; + #endregion + #region Home Decoration public const int HomeDecorationMaxLevel = 10; + #endregionMaple2.File.Ingest/Mapper/TableMapper.cs (1)
1513-1522: Consider adding XML documentation and using target-typed new expressions.The implementation is solid, but could benefit from:
- XML documentation to describe the method's purpose and return value
- Using C# 9.0's target-typed new expression for better readability
Here's the suggested improvement:
+ /// <summary> + /// Parses the masteryugchousing.xml file into a strongly-typed table. + /// </summary> + /// <returns>A table containing UGC housing mastery data.</returns> private MasteryUgcHousingTable ParseMasteryUgcHousingTable() { - var results = new Dictionary<int, MasteryUgcHousingTable.Entry>(); + Dictionary<int, MasteryUgcHousingTable.Entry> results = new(); foreach ((int id, MasteryUgcHousing ugcHousing) in parser.ParseMasteryUgcHousing()) { results.Add(id, new MasteryUgcHousingTable.Entry( Level: ugcHousing.grade,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Maple2.Server.World/Migrations/20250207213214_HomeDecoration.Designer.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/20250207213214_HomeDecoration.csis excluded by!Maple2.Server.World/Migrations/*Maple2.Server.World/Migrations/Ms2ContextModelSnapshot.csis excluded by!Maple2.Server.World/Migrations/*
📒 Files selected for processing (14)
Maple2.Database/Model/Map/Home.cs(4 hunks)Maple2.Database/Storage/Game/GameStorage.ServerInfo.cs(1 hunks)Maple2.Database/Storage/Metadata/TableMetadataStorage.cs(3 hunks)Maple2.File.Ingest/Maple2.File.Ingest.csproj(1 hunks)Maple2.File.Ingest/Mapper/TableMapper.cs(2 hunks)Maple2.Model/Game/User/Home.cs(2 hunks)Maple2.Model/Metadata/Constants.cs(1 hunks)Maple2.Model/Metadata/Table/MasteryUgcHousingTable.cs(1 hunks)Maple2.Model/Metadata/TableMetadata.cs(1 hunks)Maple2.Server.Game/Commands/HomeCommand.cs(1 hunks)Maple2.Server.Game/Manager/HousingManager.cs(3 hunks)Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs(3 hunks)Maple2.Server.Game/Packets/CubePacket.cs(1 hunks)Maple2.Server.Game/Session/GameSession.cs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- Maple2.File.Ingest/Maple2.File.Ingest.csproj
🧰 Additional context used
📓 Learnings (1)
Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs (1)
Learnt from: AngeloTadeucci
PR: AngeloTadeucci/Maple2#236
File: Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs:375-422
Timestamp: 2024-11-12T15:13:52.918Z
Learning: In `RequestCubeHandler`, the `HandleLoadLayout` method is responsible for applying the layout to the plot after missing cubes are calculated in `HandleRequestLayout`.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (12)
Maple2.Model/Metadata/Table/MasteryUgcHousingTable.cs (1)
3-8: LGTM! Well-structured table definition.The record is well-designed with appropriate use of
IReadOnlyDictionaryfor immutable data and clear property naming.Maple2.Database/Storage/Game/GameStorage.ServerInfo.cs (1)
36-36: LGTM! Consistent with other daily reset operations.The SQL command is properly placed within the transaction context and follows the same pattern as other reset operations.
Maple2.Server.Game/Commands/HomeCommand.cs (1)
53-57: Add validation for decoration level.The
LevelCommand.Handlemethod should validate the input level to ensure it's within acceptable bounds.Consider adding validation:
private void Handle(InvocationContext context, int level) { + if (level < 0) { + throw new ArgumentException("Decoration level cannot be negative.", nameof(level)); + } Home home = session.Player.Value.Home; home.DecorationLevel = level; session.Send(CubePacket.DesignRankReward(home)); }Maple2.Database/Model/Map/Home.cs (1)
23-23: LGTM! Well-structured property and configuration.The
InteriorRewardsClaimedproperty is well-defined with proper JSON conversion configuration.Also applies to: 106-106
Maple2.Model/Metadata/TableMetadata.cs (1)
81-81: LGTM! The derived type declaration follows the established pattern.The new derived type for
MasteryUgcHousingTableis correctly configured with a matching type discriminator that aligns with the XML file naming convention.Maple2.Model/Game/User/Home.cs (1)
157-169: LGTM! The experience gain logic is well-implemented.The method includes:
- Proper validation for positive experience and max level
- Correct level-up logic using the mastery table
- Timestamp update for reward tracking
Maple2.Database/Storage/Metadata/TableMetadataStorage.cs (1)
37-37: LGTM! The table storage implementation follows the established pattern.The new
MasteryUgcHousingTableis correctly implemented with lazy loading, matching the pattern used for other tables in the storage.Also applies to: 96-96
Maple2.Server.Game/Packets/CubePacket.cs (1)
372-380: LGTM! The packet serialization is properly implemented.The method correctly writes all required fields in the appropriate order:
- Account ID
- Decoration reward timestamp
- Decoration level and experience
- Interior rewards collection
Maple2.Server.Game/Session/GameSession.cs (2)
498-500: LGTM!The daily reset for home decoration rewards follows the established pattern of other daily resets in the method.
399-401: LGTM!The packet sending for home design updates is placed appropriately in the method flow, after field entry and before other UI-related packets.
Maple2.Server.Game/PacketHandlers/RequestCubeHandler.cs (1)
44-45: LGTM!The new command enum values follow the established naming convention and are appropriately placed.
Maple2.File.Ingest/Mapper/TableMapper.cs (1)
77-77: LGTM! The new table mapping follows the established pattern.The addition of the masteryugchousing.xml mapping is consistent with other table mappings in the method.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Zin looked at this and said: "wtf this is so trash, fix it now you stupid :spit:"
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Maple2.Server.Game/Manager/HousingManager.cs (1)
369-469: 🛠️ Refactor suggestionAdd error handling and logging for inventory space checks.
The method silently returns if there's not enough inventory space. Consider adding error messages to inform the user.
public void InteriorCheckIn(Plot plot) { if (session.Item.Inventory.FreeSlots(InventoryType.Misc) < 1 || session.Item.Inventory.FreeSlots(InventoryType.Fragment) < 1) { + logger.Warning("Not enough inventory space for rewards"); + session.Send(NoticePacket.Message(StringCode.s_err_not_enough_inventory_space)); return; }Add validation for decoration score calculation.
The decoration score calculation could benefit from additional validation and error handling.
int decorationScore = 0; + if (!plot.Cubes.Any()) { + logger.Warning("No decorations found in plot"); + session.Send(NoticePacket.Message(StringCode.s_err_no_decorations)); + return; + } foreach (KeyValuePair<HousingCategory, int> goal in decorationGoal) {Add error handling for reward item creation.
The method should handle cases where item creation fails more gracefully.
🧹 Nitpick comments (1)
Maple2.Server.Game/Manager/HousingManager.cs (1)
355-368: Consider using an enum-based configuration for decoration goals.The hardcoded dictionary could be moved to a configuration file or table for easier maintenance and updates.
- private readonly Dictionary<HousingCategory, int> decorationGoal = new() { - { HousingCategory.Bed, 1 }, - { HousingCategory.Table, 1 }, - { HousingCategory.SofasChairs, 2 }, - { HousingCategory.Storage, 1 }, - { HousingCategory.WallDecoration, 1 }, - { HousingCategory.WallTiles, 3 }, - { HousingCategory.Bathroom, 1 }, - { HousingCategory.Lighting, 1 }, - { HousingCategory.Electronics, 1 }, - { HousingCategory.Fences, 2 }, - { HousingCategory.NaturalTerrain, 4 }, - }; + private readonly Dictionary<HousingCategory, int> decorationGoal; + + private void InitializeDecorationGoals() { + decorationGoal = serverTableMetadata.DecorationGoalsTable.ToDictionary( + entry => entry.Category, + entry => entry.RequiredCount + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Maple2.Database/Storage/Metadata/TableMetadataStorage.cs(3 hunks)Maple2.File.Ingest/Maple2.File.Ingest.csproj(1 hunks)Maple2.File.Ingest/Mapper/TableMapper.cs(2 hunks)Maple2.Model/Metadata/Constants.cs(1 hunks)Maple2.Model/Metadata/Table/UgcHousingPointRewardTable.cs(1 hunks)Maple2.Model/Metadata/TableMetadata.cs(1 hunks)Maple2.Server.Game/Manager/HousingManager.cs(5 hunks)Maple2.Server.Game/Session/GameSession.cs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Maple2.Model/Metadata/Constants.cs
- Maple2.File.Ingest/Maple2.File.Ingest.csproj
- Maple2.Model/Metadata/TableMetadata.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (13)
Maple2.Model/Metadata/Table/UgcHousingPointRewardTable.cs (1)
1-7: LGTM! Clean and well-structured data model.The record implementation with immutable properties is a good choice for this data structure. The class hierarchy and property naming are clear and appropriate.
Maple2.Database/Storage/Metadata/TableMetadataStorage.cs (4)
37-38: LGTM! Consistent implementation of table storage.The new table fields and properties follow the established pattern for lazy loading and access. The implementation maintains consistency with the existing codebase.
Also applies to: 97-98, 158-159
37-38: LGTM!The new fields follow the established pattern of using
Lazy<T>for lazy loading of metadata tables, and the naming convention is consistent with other fields in the class.
97-98: LGTM!The new properties follow the established pattern and provide consistent access to the lazy-loaded values.
158-159: LGTM!The initialization of the new tables follows the established pattern and is placed in a logical order with related tables.
Maple2.File.Ingest/Mapper/TableMapper.cs (3)
1539-1548: LGTM! Clean implementation of mastery table parsing.The parsing logic follows the established pattern and correctly maps the data to the dictionary.
77-78: LGTM!The new entries in the
Mapmethod follow the established pattern and are placed in a logical order with related tables.
1539-1548: LGTM!The
ParseMasteryUgcHousingTablemethod follows the established pattern and correctly maps all required fields from the XML data.Maple2.Server.Game/Session/GameSession.cs (3)
151-151: LGTM!The
HousingManagerconstructor now includesServerTableMetadataas required by the home decoration feature.
399-401: LGTM!The
EnterFieldmethod now sends the necessary packets to update the client with home decoration information:
- Design rank reward status
- Profile updates
- Return map information
499-500: LGTM!The
DailyResetmethod now resets the decoration reward timestamp and sends an update packet to the client.Maple2.Server.Game/Manager/HousingManager.cs (2)
27-28: LGTM!The
HousingManagerclass now includesServerTableMetadatafor accessing server-side table data.Also applies to: 34-38
342-343: LGTM!The
InitNewHomemethod now sets the housing category for plot cubes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Maple2.Server.Game/Manager/HousingManager.cs (1)
434-455: 🛠️ Refactor suggestionAdd validation for reward ID.
The method should validate the reward ID before processing.
Add error handling and logging.
The method should handle errors more gracefully and provide feedback to the user.
Apply this diff to improve error handling:
public void InteriorReward(byte rewardId) { if (Home.InteriorRewardsClaimed.Contains(rewardId)) { + logger.Information("Reward already claimed: {RewardId}", rewardId); + session.Send(NoticePacket.Message(StringCode.s_err_already_received_reward, NoticePacket.Flags.Message)); return; } tableMetadata.MasteryUgcHousingTable.Entries.TryGetValue(rewardId, out MasteryUgcHousingTable.Entry? reward); if (reward == null) { + logger.Error("Invalid reward ID: {RewardId}", rewardId); + session.Send(NoticePacket.Message(StringCode.s_err_invalid_reward, NoticePacket.Flags.Message)); return; } Item? rewardItem = session.Field.ItemDrop.CreateItem(reward.RewardJobItemId); if (rewardItem == null) { + logger.Error("Failed to create reward item {ItemId}", reward.RewardJobItemId); + session.Send(NoticePacket.Message(StringCode.s_err_failed_to_create_reward, NoticePacket.Flags.Message)); return; } if (!session.Item.Inventory.Add(rewardItem, true)) { + logger.Warning("Failed to add reward item {ItemId} to inventory", reward.RewardJobItemId); + session.Send(NoticePacket.Message(StringCode.s_err_not_enough_inven_space, NoticePacket.Flags.Message)); return; }
🧹 Nitpick comments (4)
Maple2.Server.Tests/Tools/WeightedSetTests.cs (2)
9-15: Enhance AddTest coverage.While the test verifies the count, consider enhancing it to:
- Verify the actual items and weights using the IEnumerable interface
- Test duplicate additions
[Test] public void AddTest() { var set = new WeightedSet<int>(); set.Add(1, 1); set.Add(2, 2); set.Add(3, 3); Assert.That(set, Has.Count.EqualTo(3)); + // Verify items and weights + var expected = new[] { (1, 1), (2, 2), (3, 3) }; + Assert.That(set, Is.EquivalentTo(expected)); + + // Test duplicate addition + set.Add(1, 1); + Assert.That(set, Has.Count.EqualTo(3)); }
7-7: Add test cases for edge cases and thread safety.Consider adding tests for:
- Single item in set
- Zero weights
- Thread safety (since Interlocked is used in Add)
[Test] public void SingleItemTest() { var set = new WeightedSet<int>(); set.Add(1, 1); Assert.That(set.Get(), Is.EqualTo(1)); } [Test] public void ZeroWeightTest() { var set = new WeightedSet<int>(); set.Add(1, 0); set.Add(2, 1); // Should never return item with zero weight for (int i = 0; i < 1000; i++) { Assert.That(set.Get(), Is.EqualTo(2)); } } [Test] public void ThreadSafetyTest() { var set = new WeightedSet<int>(); var tasks = new List<Task>(); for (int i = 0; i < 10; i++) { int value = i; tasks.Add(Task.Run(() => set.Add(value, 1))); } Task.WaitAll(tasks.ToArray()); Assert.That(set, Has.Count.EqualTo(10)); }Maple2.Server.Game/Manager/HousingManager.cs (2)
353-365: Consider moving decoration goals to configuration.While the goals are well-defined and immutable, hardcoding them makes it difficult to adjust for game balance. Consider moving these values to a configuration file or table metadata for easier tuning.
367-432: Add error handling and logging for inventory space check.The method has good error handling for reward retrieval but could benefit from logging when inventory space is insufficient.
Apply this diff to improve error handling:
public void InteriorCheckIn(Plot plot) { if (session.Item.Inventory.FreeSlots(InventoryType.Misc) < 1 || session.Item.Inventory.FreeSlots(InventoryType.Fragment) < 1) { + logger.Warning("Insufficient inventory space for interior check-in rewards"); + session.Send(NoticePacket.Message(StringCode.s_err_not_enough_inven_space, NoticePacket.Flags.Message)); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Maple2.Server.Game/Manager/HousingManager.cs(4 hunks)Maple2.Server.Game/Manager/Items/ItemDropManager.cs(1 hunks)Maple2.Server.Game/Session/GameSession.cs(2 hunks)Maple2.Server.Tests/Tools/WeightedSetTests.cs(1 hunks)Maple2.Tools/WeightedSet.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: format
- GitHub Check: build
🔇 Additional comments (6)
Maple2.Tools/WeightedSet.cs (1)
30-30: LGTM! Improved probability distribution.The change from
<=to<fixes the probability distribution. Now items with weight W have exactly W/totalWeight probability of being selected, which is the correct behavior for a weighted random selection.Maple2.Server.Tests/Tools/WeightedSetTests.cs (1)
18-37: LGTM! Well-designed statistical tests.The statistical tests are well-implemented with:
- Sufficient iterations (100,000) for statistical significance
- Reasonable deviation allowance (1%)
- Good coverage of both weighted and equal distribution cases
Also applies to: 40-59
Maple2.Server.Game/Manager/Items/ItemDropManager.cs (1)
132-132: LGTM! Simplified control flow.The change improves readability by directly returning a new list when
selectedItemis null, eliminating the need for an intermediate variable.Maple2.Server.Game/Manager/HousingManager.cs (1)
302-302: LGTM! Improved code organization.The changes enhance maintainability by:
- Centralizing home initialization logic in
NewHomeDefaults- Setting the housing category for plot cubes, which is required for decoration tracking
Also applies to: 340-341
Maple2.Server.Game/Session/GameSession.cs (2)
399-401: LGTM! The sequence of home-related packet sends is logical.The packets are sent in the correct order, updating the client with home decoration information early in the field entry process.
498-500: Consider resetting additional home decoration state.While resetting the decoration reward timestamp is correct, consider if other home decoration state (like
InteriorRewardsClaimed) should also be reset during the daily reset.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if there are other home-related fields that might need daily reset # Search for other timestamp or claimed reward fields in the Home class ast-grep --pattern 'class Home { $$$ $field: $_ $$$ }'Length of output: 62
🏁 Script executed:
#!/bin/bash # Search for the "InteriorRewardsClaimed" field in the codebase to verify its existence and usage. rg "InteriorRewardsClaimed" -n .Length of output: 1578
Action Required: Reset Home Interior Reward State as Needed
The codebase confirms that the Home class maintains an InteriorRewardsClaimed field that tracks claimed rewards. The daily reset logic in GameSession.cs only resets the DecorationRewardTimestamp, leaving InteriorRewardsClaimed unchanged. If the intended design is to reset all home decoration state on a daily basis, you should also reset (or clear) InteriorRewardsClaimed.
- The Home class in Maple2.Database/Model/Map/Home.cs and Maple2.Model/Game/User/Home.cs defines InteriorRewardsClaimed.
- The daily reset in Maple2.Server.Game/Session/GameSession.cs currently only resets DecorationRewardTimestamp.
Review the business requirements to decide whether InteriorRewardsClaimed should also be reset during the daily reset.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Maple2.Server.Game/Manager/HousingManager.cs (3)
353-365: Consider moving decoration goals to configuration.While the goals are well-defined, hardcoding them in the source code makes it difficult to adjust values without recompilation. Consider moving these to a configuration file or database table for easier maintenance.
- private readonly Dictionary<HousingCategory, int> decorationGoal = new() { - { HousingCategory.Bed, 1 }, - { HousingCategory.Table, 1 }, - { HousingCategory.SofasChairs, 2 }, - { HousingCategory.Storage, 1 }, - { HousingCategory.WallDecoration, 1 }, - { HousingCategory.WallTiles, 3 }, - { HousingCategory.Bathroom, 1 }, - { HousingCategory.Lighting, 1 }, - { HousingCategory.Electronics, 1 }, - { HousingCategory.Fences, 2 }, - { HousingCategory.NaturalTerrain, 4 }, + private readonly Dictionary<HousingCategory, int> decorationGoal; + + public HousingManager(GameSession session, TableMetadataStorage tableMetadata) { + this.session = session; + this.tableMetadata = tableMetadata; + this.decorationGoal = tableMetadata.DecorationGoalsTable.ToDictionary(); + }
367-370: Consider using a more efficient counting approach.The LINQ query with
GroupByandToDictionarycould be replaced with a more efficient counting approach.- Dictionary<HousingCategory, int> decorationCurrent = plot.Cubes.Values.GroupBy(plotCube => plotCube.HousingCategory) - .ToDictionary(grouping => grouping.Key, grouping => grouping.Count()); + var decorationCurrent = new Dictionary<HousingCategory, int>(); + foreach (var cube in plot.Cubes.Values) { + if (!decorationCurrent.ContainsKey(cube.HousingCategory)) { + decorationCurrent[cube.HousingCategory] = 0; + } + decorationCurrent[cube.HousingCategory]++; + }
382-403: Simplify scoring logic using a lookup table.The switch statement for scoring could be simplified using a lookup table.
+ private static readonly Dictionary<HousingCategory, int> CategoryScores = new() { + { HousingCategory.Bed, 100 }, + { HousingCategory.SofasChairs, 100 }, + { HousingCategory.WallDecoration, 100 }, + { HousingCategory.WallTiles, 100 }, + { HousingCategory.Bathroom, 100 }, + { HousingCategory.Lighting, 100 }, + { HousingCategory.Fences, 100 }, + { HousingCategory.NaturalTerrain, 100 }, + { HousingCategory.Table, 50 }, + { HousingCategory.Storage, 50 }, + { HousingCategory.Electronics, 200 }, + }; + - switch (goal.Key) { - case HousingCategory.Bed: - case HousingCategory.SofasChairs: - case HousingCategory.WallDecoration: - case HousingCategory.WallTiles: - case HousingCategory.Bathroom: - case HousingCategory.Lighting: - case HousingCategory.Fences: - case HousingCategory.NaturalTerrain: - decorationScore += 100; - break; - case HousingCategory.Table: - case HousingCategory.Storage: - decorationScore += 50; - break; - case HousingCategory.Electronics: - decorationScore += 200; - break; - default: - continue; - } + if (CategoryScores.TryGetValue(goal.Key, out int score)) { + decorationScore += score; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Maple2.Server.Game/Manager/HousingManager.cs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
Maple2.Server.Game/Manager/HousingManager.cs (1)
301-351: LGTM! Good refactoring of home initialization.The changes improve code organization by centralizing home initialization logic in
NewHomeDefaultsand properly tracking housing categories.
2 participants
Summary by CodeRabbit
New Features
• Upgraded home customization with enhanced decoration leveling, experience tracking, and interior reward management.
• Introduced new in-game commands allowing players to update and manage decoration values while claiming tailored rewards.
• Added new constants related to home decoration.
• Implemented new records for mastery and housing point rewards to enhance reward management.
• Added functionality for checking in and claiming rewards based on decoration achievements.
Refactor
• Streamlined home initialization to automatically set default values.
• Improved real-time updates of home decoration status at daily resets and when entering game fields.