Refactor Lua - #447
Conversation
Removed Maple2.Lua Added C# conversion of used lua functions
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThese changes remove the direct dependency on the Changes
Sequence Diagram(s)sequenceDiagram
participant GameSession
participant Manager
participant LuaFunctions
GameSession->>Manager: Instantiate (no Lua.Lua passed)
Manager->>LuaFunctions: Call Static Lua Function
LuaFunctions-->>Manager: Return Calculation Result
Manager-->>GameSession: Use Result in Logic
sequenceDiagram
participant TestSuite
participant Maple2Lua
participant LuaFunctions
TestSuite->>Maple2Lua: Call Lua Interpreter Function
TestSuite->>LuaFunctions: Call Static C# Lua Function
Maple2Lua-->>TestSuite: Return Result
LuaFunctions-->>TestSuite: Return Result
TestSuite->>TestSuite: Assert Results Match
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Maple2.Server.Game/Model/Stats.cs (1)
5-5: Remove unusedSerilogimport
Theusing Serilog;directive is added but no Serilog APIs are utilized in this class. Either remove the import to keep the code clean or add the intended logging statements (e.g., in constructors or key methods) to leverage Serilog here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (24)
Maple2.Server.DebugGame/Program.cs(0 hunks)Maple2.Server.Game/Manager/BlackMarketManager.cs(4 hunks)Maple2.Server.Game/Manager/ExperienceManager.cs(3 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs(2 hunks)Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs(0 hunks)Maple2.Server.Game/Manager/Field/FieldManager/IField.cs(0 hunks)Maple2.Server.Game/Manager/ItemEnchantManager.cs(1 hunks)Maple2.Server.Game/Manager/MasteryManager.cs(3 hunks)Maple2.Server.Game/Manager/StatsManager.cs(5 hunks)Maple2.Server.Game/Maple2.Server.Game.csproj(0 hunks)Maple2.Server.Game/Model/Field/Actor/Actor.cs(0 hunks)Maple2.Server.Game/Model/Stats.cs(1 hunks)Maple2.Server.Game/PacketHandlers/ChangeAttributesHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/ItemDismantleHandler.cs(0 hunks)Maple2.Server.Game/PacketHandlers/ItemSocketHandler.cs(1 hunks)Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/RevivalHandler.cs(2 hunks)Maple2.Server.Game/PacketHandlers/TaxiHandler.cs(1 hunks)Maple2.Server.Game/Program.cs(0 hunks)Maple2.Server.Game/Session/GameSession.cs(2 hunks)Maple2.Server.Game/Util/ItemStatsCalculator.cs(1 hunks)Maple2.Server.Tests/Lua/LuaTests.cs(1 hunks)Maple2.Server.Tests/Maple2.Server.Tests.csproj(1 hunks)
💤 Files with no reviewable changes (7)
- Maple2.Server.Game/Maple2.Server.Game.csproj
- Maple2.Server.Game/Program.cs
- Maple2.Server.DebugGame/Program.cs
- Maple2.Server.Game/Manager/Field/FieldManager/IField.cs
- Maple2.Server.Game/PacketHandlers/ItemDismantleHandler.cs
- Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.cs
- Maple2.Server.Game/Model/Field/Actor/Actor.cs
🧰 Additional context used
🧬 Code Graph Analysis (2)
Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (1)
Maple2.Server.Game/LuaFunctions/Lua.cs (3)
Lua(11-3180)GetMesoRevivalDailyMaxCount(834-836)CalcRevivalPrice(543-549)
Maple2.Server.Tests/Lua/LuaTests.cs (2)
Maple2.Server.Game/LuaFunctions/Lua.cs (11)
Lua(11-3180)CalcItemLevel(76-460)CalcCritDamage(44-50)CalcPlayerCritRate(52-66)CalcNpcCritRate(68-74)CalcRevivalPrice(543-549)CalcTaxiCharge(786-810)CalcAirTaxiCharge(812-814)ConstantValueCap(1935-1947)ConstantValueWapMax(2231-2286)ConstantValueWapMin(2174-2229)Maple2.Model/Game/Item/ItemType.cs (1)
ItemType(4-4)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (49)
Maple2.Server.Game/Manager/Field/FieldManager/FieldManager.State.cs (2)
11-11: Added required namespace for static Lua functions.The new import allows access to the static Lua functions that replace the previous instance-based Lua interpreter.
329-329: Added explicit cast to int for Lua calculation result.This explicit cast clarifies that the static
Lua.CalcNpcSpawnWeightfunction returns a non-integer type that needs to be converted to an integer for theWeightedSet.Add()method. The change is part of the larger Lua refactoring to use static methods instead of instance methods.Maple2.Server.Tests/Maple2.Server.Tests.csproj (1)
11-11: Moved Lua dependency to test project.As part of the Lua refactoring, the direct dependency on the Lua interpreter has been moved from the main project to the test project. This supports the new test coverage that validates that the static Lua function implementations match the behavior of the original Lua interpreter.
Maple2.Server.Game/PacketHandlers/HomeDoctorHandler.cs (2)
7-7: Added required namespace for static Lua functions.The new import enables access to the static Lua functions that replace the previous instance-based approach.
25-25: Replaced Lua instance method with static method call.The code now uses the static
Lua.CalcResolvePenaltyPricemethod instead of accessing it through asession.Luainstance. This is part of the broader refactoring to remove direct dependencies on Lua interpreter instances.Maple2.Server.Game/PacketHandlers/ResolveDeathPenaltyHandler.cs (2)
5-5: Added required namespace for static Lua functions.The import allows access to the static Lua class, aligning with the refactoring approach.
25-25: Replaced Lua instance method with static method call.Similar to the changes in HomeDoctorHandler, this change replaces the instance-based Lua method call with a static method call, maintaining the same functionality while removing the dependency on the Lua interpreter instance.
Maple2.Server.Game/PacketHandlers/ChangeAttributesHandler.cs (2)
8-8: LGTM: Added using directive for static Lua functions.The new using directive properly supports the transition from instance-based Lua interpreter to static Lua function calls.
104-105:Details
✅ Verification successful
Verify that static Lua function calls work correctly for all code paths.
The code now uses static
Lua.CalcGetPetRemakeIngredientandLua.CalcGetItemRemakeIngredientNewmethods instead of the previous instance-based approach. This change aligns with the refactoring to remove direct Lua interpreter dependencies.
🏁 Script executed:
#!/bin/bash # Check for any instances where Lua is still used as an instance variable rg "\bLua\." --type=cs | grep -v "Maple2\.Server\.Game\.LuaFunctions\.Lua\."Length of output: 8838
🏁 Script executed:
#!/bin/bash # Locate static method definitions in LuaFunctions.Lua rg -n "CalcGetPetRemakeIngredient" --type=cs rg -n "CalcGetItemRemakeIngredientNew" --type=cs # Verify no production-level Lua interpreter instances remain outside tests rg -n "new Maple2.Lua.Lua" --type=cs | grep -v "Maple2.Server.Tests"Length of output: 873
Static Lua method calls verified
CalcGetPetRemakeIngredient(line 935) andCalcGetItemRemakeIngredientNew(line 871) are defined asstaticinMaple2.Server.Game/LuaFunctions/Lua.cs.- No
new Maple2.Lua.Luainstantiations remain in production code (only in tests).No further changes required.
Maple2.Server.Game/PacketHandlers/ItemSocketHandler.cs (3)
9-9: LGTM: Added using directive for static Lua functions.The new using directive properly supports the transition from instance-based Lua interpreter to static Lua function calls.
117-117: LGTM: Converted to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call in the
CalcItemSocketUnlockIngredientfunction.
329-329: LGTM: Converted to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call in the
CalcGetGemStonePutOffPricefunction.Maple2.Server.Game/Util/ItemStatsCalculator.cs (5)
9-9: LGTM: Added using directive for static Lua functions.The new using directive properly supports the transition from instance-based Lua interpreter to static Lua function calls.
142-142: LGTM: Converted to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call in the
CalcItemSocketMaxCountfunction.
609-622: LGTM: Converted ConstantValue methods to static Lua function calls.Successfully transitioned from instance-based Lua interpreter calls to static method calls for all the constant value calculations. This change simplifies the dependency structure.
627-634: LGTM: Converted StaticValue methods to static Lua function calls.Successfully transitioned from instance-based Lua interpreter calls to static method calls for all the static value calculations. This change maintains functionality while removing direct Lua dependency.
641-644: LGTM: Converted StaticRate method to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call for the static rate calculation.
Maple2.Server.Game/PacketHandlers/TaxiHandler.cs (3)
9-9: LGTM: Added using directive for static Lua functions.The new using directive properly supports the transition from instance-based Lua interpreter to static Lua function calls.
83-83: LGTM: Converted to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call in the
CalcTaxiChargefunction.
117-117: LGTM: Converted to static Lua function call.Successfully transitioned from instance-based Lua interpreter call to static method call in the
CalcAirTaxiChargefunction.Maple2.Server.Game/Manager/ItemEnchantManager.cs (1)
69-69: Approval for Lua dependency removalThe constructor signature has been simplified to remove the Lua interpreter instance dependency, which aligns with the broader refactoring initiative to centralize Lua function calls into static methods.
Maple2.Server.Game/PacketHandlers/RevivalHandler.cs (3)
7-7: Correctly added LuaFunctions namespace importThe import of the Maple2.Server.Game.LuaFunctions namespace is necessary for accessing the static Lua methods that replace the instance-based approach.
62-62: Properly converted to static Lua function callChanged from using the session.Lua instance method to the static implementation, maintaining the same functionality while eliminating the dependency on the Lua interpreter instance.
67-67: Correctly migrated to static Lua function callThe method call for calculating revival price has been updated to use the static version, preserving the same behavior but removing the dependency on the session's Lua instance.
Maple2.Server.Game/Session/GameSession.cs (4)
160-160: Constructor parameter removed from MasteryManager initializationCorrectly updated to initialize MasteryManager without the Lua parameter, consistent with the refactoring goal of removing direct Lua interpreter dependencies.
165-165: ItemEnchantManager dependency simplifiedThe ItemEnchantManager initialization no longer requires the Lua parameter, aligning with the updated constructor signature in that class.
170-170: ExperienceManager initialization updatedProperly removed the Lua parameter from the ExperienceManager constructor call, matching the refactored implementation.
179-179: BlackMarketManager instantiation simplifiedCorrectly updated to initialize BlackMarketManager without passing a Lua instance, completing the dependency removal pattern across all manager classes.
Maple2.Server.Game/Manager/MasteryManager.cs (3)
8-8: Added required LuaFunctions namespaceThe import of Maple2.Server.Game.LuaFunctions is necessary to access the static Lua methods that replace the instance-based approach.
22-24: Simplified constructor by removing Lua dependencyThe constructor signature has been updated to remove the Lua.Lua parameter, aligning with the broader refactoring initiative.
133-133: Converted to static Lua function callThe gathering success rate calculation has been correctly migrated from an instance method call on the lua object to a static method call on the Lua class, maintaining the same functionality while eliminating the interpreter instance dependency.
Maple2.Server.Game/Manager/BlackMarketManager.cs (4)
8-8: Added appropriate namespace import for Lua static functions.This import ensures access to the static Lua functions that replace the instance-based calls, aligning with the broader refactoring to remove direct Lua interpreter dependencies.
21-23: Removed Lua instance dependency from constructor.The constructor no longer accepts a Lua parameter, which is consistent with the project-wide refactoring to eliminate Lua instance dependencies from manager classes.
48-49: Replaced instance Lua calls with static method calls.The deposit percentage and fee calculations now use static Lua method calls instead of instance methods, maintaining the same functionality while eliminating the direct Lua interpreter dependency.
271-271: Replaced instance Lua call with static method call for cost rate calculation.The black market cost rate calculation now uses a static Lua method call consistent with other Lua function calls in this class.
Maple2.Server.Game/Manager/ExperienceManager.cs (3)
5-5: Added appropriate namespace import for Lua static functions.This import provides access to the static Lua functions required for experience calculations.
55-58: Removed Lua instance dependency from constructor.The constructor has been simplified to remove the Lua parameter, aligning with the project-wide refactoring to eliminate direct Lua interpreter dependencies.
77-77: Replaced instance Lua call with static method call for kill count bonus experience calculation.The kill count bonus experience rate calculation now uses a static Lua method call, maintaining the same functionality while eliminating the direct Lua interpreter dependency.
Maple2.Server.Game/Manager/StatsManager.cs (5)
6-6: Added appropriate namespace import for Lua static functions.This import provides access to the static Lua functions required for stats calculations.
91-91: Replaced instance Lua calls with static method call for critical damage calculation.The critical damage calculation now uses a static Lua method call instead of instance methods on player or NPC objects, maintaining the same functionality while eliminating the direct Lua interpreter dependency.
106-108: Replaced instance Lua calls with static method calls for critical rate calculations.Both player and NPC critical rate calculations now use static Lua method calls instead of instance methods, maintaining the same functionality while eliminating the direct Lua interpreter dependencies.
161-161: Replaced instance Lua call with static method call for item level calculation.The item level calculation now uses a static Lua method call, maintaining the same functionality while eliminating the direct Lua interpreter dependency.
209-211: Added logging for critical damage attribute.This new logging statement will help with debugging critical damage calculations by providing visibility into when these attributes are added and their values.
Maple2.Server.Tests/Lua/LuaTests.cs (6)
1-14: Well-structured test class setup with appropriate NonParallelizable attribute.The test class is properly set up with:
- Required imports
- NonParallelizable attribute to prevent concurrent test execution
- Lua interpreter instance initialization
- Proper cleanup in the OneTimeTearDown method
This structure ensures reliable and consistent test execution for validating Lua function implementations.
15-31: Comprehensive test for CalcItemLevel function.This test validates that the static implementation of CalcItemLevel produces the same results as the original Lua interpreter implementation, using a realistic example with appropriate gear score, rarity and item type values.
33-50: Well-parameterized tests for CalcCritDamage function.This test case uses multiple parameter combinations to verify that critical damage calculations match between the static C# implementation and the original Lua interpreter across different input values and modes. The test includes boundary cases and confirms proper clamping behavior.
52-76: Thorough validation of critical rate calculation functions.These tests verify both player and NPC critical rate calculations using realistic parameter values, ensuring the static implementations match the original Lua interpreter results with appropriate precision tolerance.
78-124: Comprehensive tests for revival price and taxi charge calculations.These test cases cover various revival price and taxi charge scenarios including:
- Boundary cases (level 1, 8, etc.)
- Various distance and level combinations for taxi charges
- Air taxi charges across different level ranges
All tests ensure the static implementations match the original Lua interpreter results exactly.
126-163: Complete test coverage for constant value calculations.These tests validate the complex weapon and item stat calculations including:
- ConstantValueCap
- ConstantValueWapMax (weapon attack power maximum)
- ConstantValueWapMin (weapon attack power minimum)
The tests use realistic item parameters (likely from an actual game item - "fire prism star") and ensure the static implementations match the original Lua interpreter results with appropriate precision tolerance.
Summary by CodeRabbit
Refactor
Tests
Chores