Object-oriented refactor of Suggest Gems #4083
Conversation
to facilitate flexible use by other sims. Changes to be committed: new file: ui/core/components/suggest_gems_action.ts modified: ui/core/proto_utils/equipped_item.ts modified: ui/core/proto_utils/gear.ts modified: ui/feral_druid/sim.ts
Changes to be committed: modified: ui/core/components/suggest_gems_action.ts
Changes to be committed: modified: ui/core/components/suggest_gems_action.ts modified: ui/core/proto_utils/gear.ts
copy-pasted code. Changes to be committed: modified: ui/warrior/sim.ts
Changes to be committed: modified: ui/warrior/sim.ts
Changes to be committed: modified: ui/hunter/sim.ts modified: ui/warrior/sim.ts
Changes to be committed: modified: ui/feral_tank_druid/sim.ts
|
@1337LutZ @TheGroxEmpire Finally finished the Suggest Gems refactor to make it object-oriented and extensible to different types of sims without copy-pasting code. Would appreciate if you can take a look before I merge. |
| this.sim = simUI.sim; | ||
|
|
||
| // Initialize empty arrays of gem priorities for each socket color | ||
| this.gemPriorityByColor = {} as Record<GemColor, Array<GemCapsData>>; |
There was a problem hiding this comment.
Cant this be a empty object by default on line 23?
protected readonly gemPriorityByColor: Record<GemColor, Array<GemCapsData>> = {};
There was a problem hiding this comment.
I wanted to initialize all the keys explicitly with empty Arrays, since I query the length elsewhere, such as on line 82.
| if (itemSlot != null) { | ||
| const item = gear.getEquippedItem(itemSlot); | ||
|
|
||
| for (const [socketIdx, socketColor] of item!.allSocketColors().entries()) { |
There was a problem hiding this comment.
instead of item! do we want do just return early if !item on line 149-150?
There was a problem hiding this comment.
My initial assumption was that item would always be non-null if itemSlot is non-null, since findStrongestSocketBonus() already has a null check in it. But I added the early return anyway just for completeness.
| this.arpStackDetected = this.detectArpStackConfiguration(ungemmedGear); | ||
|
|
||
| // Update red gem priority | ||
| const redGemCaps = new Array<GemCapsData>(); |
There was a problem hiding this comment.
The priority of adding the gems on line 328 and below can be different per class/spec. Is there a way to also make that dynamic?
There was a problem hiding this comment.
Do you have an example of a physical DPS spec that would use a different priority than the one listed here? My thought was to generalize code like this on an as-needed basis, as more sims start to implement GemOptimizer subclasses.
|
Super nice work <3 Added some comments in regards of TS / gem orders. |
|
Created a build and looks like it is gemming sometimes even better (esp prioing the tear socket) for most specs 👍 |
Changes to be committed: modified: ui/core/components/suggest_gems_action.ts
|
Will go ahead and merge this for now, and if there is a need to further compartmentalize the |
No description provided.