Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ItemAPI tier fix #394

Merged
merged 3 commits into from
Apr 25, 2022
Merged

ItemAPI tier fix #394

merged 3 commits into from
Apr 25, 2022

Conversation

Nebby1999
Copy link
Contributor

Update 1.2.3 to Risk of Rain made the itemDef's "tier" into a property (Was originally a field). the Property's Set method causes issues when the itemTier catalog as not initialized yet (set method calls ItemTierCatalog.GetItemTierDef(itemTierEnum), and at that point no itemTierDefs exist in the catalog yet.)

This update fixes this issue by doing the following check on the tierargument, while also adding a new optional argument (For itemTierDefs)

When the tier argument is not assignedAtRuntime, the ItemDef's _itemTierDef gets loaded via Addressables by doing a switch case on the given tier argument.

If the tier argument is assignedAtRuntime, but no itemTierDef is specified, R2API throws a warning that the itemDef has the tier argument to assignedAtRuntime, but itemTierDef argument is null, afterwards it sets the ItemDef._itemTierDef to null (AKA: No tier)

if both the tier is AssignedAtRuntime, and the itemTierDef argument is not null, then it simpplpy sets the itemDef's _itemTierDef to the argument's value

I am unsure if this counts as a breaking change, some testing is needed

R2API/ItemAPI.cs Outdated Show resolved Hide resolved
R2API/ItemAPI.cs Outdated Show resolved Hide resolved
R2API/ItemAPI.cs Outdated Show resolved Hide resolved
@Nebby1999
Copy link
Contributor Author

I just tried to fix this by adding 2 overloads to the constructor and having the actual construction run in a private method, may not be the cleanest way of fixing the breakage but i cant figure out another one atm

@tristanmcpherson tristanmcpherson merged commit 2ed4c49 into risk-of-thunder:master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants