Fix issue with inefficient skill to craft first seed#2
Fix issue with inefficient skill to craft first seed#2papadakis-k wants to merge 1 commit intoIdrinth:masterfrom
Conversation
When you have selected the "First" option and have a seed in your inventory that is higher than your current skill, as the first seed, then it shouldn't be selected, otherwise it causes errors on the client (and crafting stops).
WalkthroughA single-line conditional modification in the getFirst function now requires max to be greater than or equal to itemdata.craftingSkillRequirement, in addition to existing cultivationType validation. This restricts item selection until the crafting skill requirement threshold is satisfied. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
MiracleGrowLight.lua (1)
77-126: Consider extracting common predicates to reduce duplication.The cultivation type check and skill requirement validation appear in multiple functions. Extracting these into small helper functions could improve maintainability.
♻️ Optional refactoring to reduce code duplication
Consider adding helper functions at the top of the local function section:
local function isSeedOrSpore(itemdata) return itemdata.cultivationType == GameData.CultivationTypes.SPORE or itemdata.cultivationType == GameData.CultivationTypes.SEED end local function meetsSkillRequirement(itemdata, playerSkill) return playerSkill >= itemdata.craftingSkillRequirement endThen update the functions to use these helpers:
local function getSeed(items, max) local i=1; local out={}; local positions = {} for index,itemdata in pairs(items) do - if itemdata.cultivationType==GameData.CultivationTypes.SPORE or itemdata.cultivationType==GameData.CultivationTypes.SEED then - if max >= itemdata.craftingSkillRequirement then + if isSeedOrSpore(itemdata) then + if meetsSkillRequirement(itemdata, max) then -- ... rest of functionSimilar changes would apply to
getLinimentandgetFirst.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MiracleGrowLight.lua
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
MiracleGrowLight.lua (1)
119-126: LGTM! The skill requirement check fixes the reported issue.The addition of
max >= itemdata.craftingSkillRequirementprevents selecting seeds that exceed the player's skill level, which directly addresses the client errors and crafting interruption described in the PR. The logic is consistent with thegetSeedfunction (line 83) and the parentheses grouping is correct.
When you have selected the "First" option and have a seed in your inventory that is higher than your current skill, as the first seed, then it shouldn't be selected, otherwise it causes errors on the client (and crafting stops).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.