refactor: change Limit, Reward, and Loot into polymorphic traits - #1509
Open
perappu wants to merge 5 commits into
Open
refactor: change Limit, Reward, and Loot into polymorphic traits#1509perappu wants to merge 5 commits into
perappu wants to merge 5 commits into
Conversation
liwoyadan
approved these changes
Jul 3, 2026
liwoyadan
left a comment
There was a problem hiding this comment.
polymorphs AND traits...life can be so so beautiful
Contributor
|
Very nice! Requesting Limitable be added to shops since it also uses limits out of the box iirc |
Member
Author
good catch LOL I knew there was something with limits that I was forgetting. |
|
I don't have the confidence to review this myself, but this is very good work |
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
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.
I'm back at it again with another big ol' refactor! This time, I have changed the Limit, Reward, and Loot models to be actually polymorphic.
"Hey Toko, what does that even mean?"
They're not very prevalent in LK, so I'd like to take some time to explain the concept of polymorphism and how it applies to us.
In the
Limitmodel, we have an if statement that compares the value oflimit_typeto certain strings. Based on those certain strings, it will return a different model, such as anItemvs aCurrency. These relationships with "changing" model types are called a polymorphic relationship.The problem is that, all this time, we have essentially been reinventing the wheel -- and to our detriment.
The if statement with
limit_typeis evaluated at runtime. What this means is that Laravel can not pull the reward ahead of time. More or less, every time thelimit()relationship is called, it has to re-query the database. In some cases, this can lead to a massive amount of duplicate queries, when either processing limits or displaying them. It's also just generally inefficient, and requires a lot of strange workarounds.Fortunately, Laravel provides some excellent ways for us to create "one-to-many" polymorphic relationship the proper way. By using relationships such as
morphTo()instead ofbelongsTo(), we can define these relationships in a way that can be eager loaded and overall handled far more simply.As part of this, I have also added "Rewardable" and "Limitable" traits. A trait is basically just a shorthand saying, "hey, attach these methods onto this model". These allow us to load the limits and rewards attached to any given object with an actual model relationship, and should be added to anything that uses limits/rewards.
Here is a summary of all the changes that this PR makes. As always, I tried to add good inline comments explaining everything, but I figured this couldn't hurt too. I have bolded/italicized any changes that may require changes to existing extensions/custom code.
Morph Map
\App\Models\Item\Itemand\App\Models\Currency\Currencyin a polymorphic model relationship.rewardable_type, etc in existing tables to make them polymorphic.getAssetModelString()to assist with converting aliases to their model strings.getAssetModelString()as well. More details below.Limits
getAssetModelString.limit_typeto the model needs to be moved to the switch statement ingetAssetModelString.dynamichas been added to getAssetModelString() to account for the above.Limitablehas been added. The Limitable trait must be added to any model that uses limits.limitable()andobject()can be used interchangeably.getLimits()andhasLimits()methods have been changed to leverage the new relationship.limit()relationship is always eager loaded (because you're basically going to need to retrieve it every time anyway).To make it super clear what changes are required for custom limit types, here is an example with the

user_levelandcharacter_levellimit types from Claymores & Companions:Rewards
getAssetModelString()and probably don't need to do anything.Rewardablehas been added. The Rewardable trait must be added to any model that uses rewards.rewardable()andobject()can be used interchangeably.getRewards()andhasRewards()methods have been changed to leverage the new relationship.reward()relationship is always eager loaded (see above).Prompts
Loot Tables
config/loot_types.php. These are used in building the morph map. They will be added similarly to custom limit types.itemcategoryanditemcategoryrarityhas been added to getAssetModelString() to account for the above....and I think that covers everything! Thanks for taking the time to read this.