Anvil Lag Fix#131
Open
MirandaLeaptrott wants to merge 1 commit into
Open
Conversation
Applied the anvil lag fixes via caching. This was originally applied to the source at the time of last release. It is tested and working.
📝 WalkthroughWalkthroughThis PR refactors SmithingPlus to introduce recipe lookup caches for improved ingredient/output lookups, simplifies the forge anvil UI workable-temperature display, removes metal-material-dependent utilities, and updates item configuration patches to use direct pure-metal marking instead of template copying. Version is bumped to v1.9.0-dev.2. ChangesRecipe Caching and Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Author
|
I forgot to note that it also seems to fix the iron bloom lag too. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SmithingPlus/Core.cs (1)
130-135:⚠️ Potential issue | 🟠 MajorFix smithing ingredient cache indexing to match injected recipes
SmithingRecipe.IngredientandSmithingRecipe.Ingredientsare independent inputs, so recipes injected vianewRecipe.Ingredient(SmithingPlus/Core.cs 130-135) won’t be included when the cache indexes onlyrecipe.Ingredients(157-170). Indexrecipe.RecipeIngredients(the unifiedIRecipeBaseinput enumeration) instead, or ensure injected recipes populate the same input collection field the cache reads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SmithingPlus/Core.cs` around lines 130 - 135, The cache indexing currently only iterates recipe.Ingredients so recipes injected via newRecipe.Ingredient are missed; update the cache population to iterate the unified enumeration recipe.RecipeIngredients (IRecipeBase) instead of recipe.Ingredients, or change the injection site to add into the same Ingredients collection; specifically modify the code around how SmithingRecipe entries are indexed (references: SmithingRecipe.Ingredient, SmithingRecipe.Ingredients, newRecipe.Ingredient, recipe.Ingredients, recipe.RecipeIngredients, IRecipeBase) so the cache reads recipe.RecipeIngredients to include both single Ingredient and Ingredients inputs.
🧹 Nitpick comments (1)
SmithingPlus/SmithingPlus.csproj (1)
15-47: 💤 Low valueInconsistent Private casing across Reference elements.
The
Privatevalues use inconsistent casing: lowercasefalseon lines 15 and 43, but capitalizedFalseelsewhere. While MSBuild is case-insensitive for boolean values, consistent casing improves readability.♻️ Standardize to one casing (suggest capitalized for consistency with majority)
- <Private>false</Private> + <Private>False</Private>Apply to lines 15 and 43.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SmithingPlus/SmithingPlus.csproj` around lines 15 - 47, Several <Reference> elements have inconsistent casing for the <Private> value: change the lowercase "false" occurrences to the capitalized "False" to match the rest; specifically update the <Private> value inside the <Reference Include="VintagestoryLib"> and the earlier <Reference> (the one with <Private>false</Private> at line showing lowercase) so all <Private> elements use "False" for consistent readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SmithingPlus/Util/CollectibleExtensions.cs`:
- Around line 76-81: The current logic treats a non-empty
Core.SmithingRecipesByOutputCode as authoritative and returns null on any key
miss; change each occurrence (where cache is assigned from
Core.SmithingRecipesByOutputCode and you call cache.TryGetValue(collObj.Code,
out var cached)) to return cached only when TryGetValue succeeds, and otherwise
fall back to querying the live registry/lookup path (i.e., the runtime/World or
Registry method used elsewhere for recipes) so late-added recipes or stale
static caches don't become permanent false negatives; keep the static snapshot
usage for hits but ensure misses delegate to the live registry instead of
returning null (this applies to the blocks at 76-81, 91-97, and 108-114).
---
Outside diff comments:
In `@SmithingPlus/Core.cs`:
- Around line 130-135: The cache indexing currently only iterates
recipe.Ingredients so recipes injected via newRecipe.Ingredient are missed;
update the cache population to iterate the unified enumeration
recipe.RecipeIngredients (IRecipeBase) instead of recipe.Ingredients, or change
the injection site to add into the same Ingredients collection; specifically
modify the code around how SmithingRecipe entries are indexed (references:
SmithingRecipe.Ingredient, SmithingRecipe.Ingredients, newRecipe.Ingredient,
recipe.Ingredients, recipe.RecipeIngredients, IRecipeBase) so the cache reads
recipe.RecipeIngredients to include both single Ingredient and Ingredients
inputs.
---
Nitpick comments:
In `@SmithingPlus/SmithingPlus.csproj`:
- Around line 15-47: Several <Reference> elements have inconsistent casing for
the <Private> value: change the lowercase "false" occurrences to the capitalized
"False" to match the rest; specifically update the <Private> value inside the
<Reference Include="VintagestoryLib"> and the earlier <Reference> (the one with
<Private>false</Private> at line showing lowercase) so all <Private> elements
use "False" for consistent readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6273a58f-3986-4f22-9e90-eb167e9e116e
📒 Files selected for processing (12)
SmithingPlus/ClientTweaks/ShowWorkablePatches.csSmithingPlus/Common/CollectibleBehaviorRecycledBit.csSmithingPlus/Core.csSmithingPlus/Properties/launchSettings.jsonSmithingPlus/SmithingPlus.csprojSmithingPlus/Util/CollectibleExtensions.csSmithingPlus/Util/Constants.csSmithingPlus/Util/ItemStackExtensions.csSmithingPlus/assets/smithingplus/patches/compatibility/xskills-metalbit.jsonSmithingPlus/assets/smithingplus/patches/metalbit.jsonSmithingPlus/changelog.mdSmithingPlus/modinfo.json
💤 Files with no reviewable changes (2)
- SmithingPlus/Util/Constants.cs
- SmithingPlus/assets/smithingplus/patches/metalbit.json
Comment on lines
+76
to
+81
| var cache = Core.SmithingRecipesByOutputCode; | ||
| if (cache.Count > 0) | ||
| { | ||
| cache.TryGetValue(collObj.Code, out var cached); | ||
| return cached; | ||
| } |
There was a problem hiding this comment.
Don't treat a non-empty cache as a complete recipe index.
These methods return null/empty on any key miss as soon as one cache entry exists. Core.AssetsFinalize() only snapshots the registries once, so later recipe additions — or stale static cache entries that survive until the next rebuild — become permanent false negatives instead of falling back to the live registry.
Suggested fallback-on-miss change
var cache = Core.SmithingRecipesByOutputCode;
- if (cache.Count > 0)
- {
- cache.TryGetValue(collObj.Code, out var cached);
- return cached;
- }
+ if (cache.Count > 0 && cache.TryGetValue(collObj.Code, out var cached))
+ return cached;
+
return api.ModLoader
.GetModSystem<RecipeRegistrySystem>()
.SmithingRecipes
.FirstOrDefault(r => r.Output.ResolvedItemstack.Collectible.Code.Equals(collObj.Code));
var cache = Core.SmithingRecipesByIngredientCode;
- if (cache.Count > 0)
- {
- return cache.TryGetValue(collObj.Code, out var cached)
- ? cached
- : System.Linq.Enumerable.Empty<SmithingRecipe>();
- }
+ if (cache.Count > 0 && cache.TryGetValue(collObj.Code, out var cached))
+ return cached;
+
return
from recipe in api.ModLoader.GetModSystem<RecipeRegistrySystem>().SmithingRecipes
from ing in recipe.Ingredients
where ing.ResolvedItemStack is not null &&
ing.ResolvedItemStack.Collectible.Code.Equals(collObj.Code)
select recipe;
var cache = Core.GridRecipesByIngredientCode;
- if (cache.Count > 0)
- {
- return cache.TryGetValue(collObj.Code, out var cached)
- ? cached
- : System.Linq.Enumerable.Empty<GridRecipe>();
- }
+ if (cache.Count > 0 && cache.TryGetValue(collObj.Code, out var cached))
+ return cached;
+
return
from recipe in api.World.GridRecipes
where recipe.RecipeIngredients != null
from ing in recipe.RecipeIngredients
where ing is { ResolvedItemStack.Collectible: not null } &&
ing.ResolvedItemStack.Collectible.Code.Equals(collObj.Code)
select recipe;Also applies to: 91-97, 108-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SmithingPlus/Util/CollectibleExtensions.cs` around lines 76 - 81, The current
logic treats a non-empty Core.SmithingRecipesByOutputCode as authoritative and
returns null on any key miss; change each occurrence (where cache is assigned
from Core.SmithingRecipesByOutputCode and you call
cache.TryGetValue(collObj.Code, out var cached)) to return cached only when
TryGetValue succeeds, and otherwise fall back to querying the live
registry/lookup path (i.e., the runtime/World or Registry method used elsewhere
for recipes) so late-added recipes or stale static caches don't become permanent
false negatives; keep the static snapshot usage for hits but ensure misses
delegate to the live registry instead of returning null (this applies to the
blocks at 76-81, 91-97, and 108-114).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applied the anvil lag fixes via caching. This was originally applied to the source at the time of last release. It is tested and working. I had this in a couple of months ago and just reapplied it. I hope it helps. I had an issue porting it into the latest release of the code where the smithing-with-bits feature was not working, so I just went with what was working.
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
Chores