Conversation
marcoabreu
commented
Oct 11, 2025
marcoabreu
commented
- bigger icons
- full loot table icon full size when items < 12
- improved sorting
- tome icon shows correctly when there's no item class but only tome items can be dropped
- fixed duplicate entries
- removed debug message
- bigger icons - full loot table icon full size when items < 12 - improved sorting - tome icon shows correctly when there's no item class but only tome items can be dropped - fixed duplicate entries - removed debug message
| [ItemClass.Misc]: 6 | ||
| }; | ||
|
|
||
| function sortItems(itemIds: string[]): string[] { |
There was a problem hiding this comment.
Do sorting ones in findMapInitialCreepsWithDrops(). No need to sort them every time we show them.
| this.allDropsBtn.btn.clearPoints() | ||
| this.allDropsBtn.btn.setPoint( | ||
| FRAMEPOINT_BOTTOMRIGHT, | ||
| Frame.fromOrigin(ORIGIN_FRAME_COMMAND_BUTTON, 11), | ||
| FRAMEPOINT_BOTTOMRIGHT, | ||
| 0, 0 | ||
| ) |
There was a problem hiding this comment.
Can remove this. The point is already set in constructor.
| } | ||
|
|
||
| if (itemIds.length < LootTableUI.MAX_ITEMS) { | ||
| this.allDropsBtn.btn.setSize(0.04, 0.04) |
There was a problem hiding this comment.
I think 0.039 is more closer to the actual icon size
| } else { | ||
| m += `|cff00ff00[Custom drop pool]|r\n` | ||
| m += set.itemDrops.map(d => { | ||
| if (d instanceof RandomItemGroupDrop) { | ||
| return ` |cff00ff00[${d.itemGroup.itemClass}, Level ${d.itemGroup.itemLevel}]|r\n` + | ||
| d.getDropItemIds() | ||
| .map(id => ` - ${getItemById(id).name}`) | ||
| .join("\n") | ||
| } else { | ||
| return ` ${getItemById(d.getRawId()).name}` | ||
| } | ||
| }).join("\n") | ||
| m += `|cff00ff00[Custom]|r\n` | ||
|
|
||
| const allIds = Array.from( | ||
| new Set( | ||
| set.itemDrops.flatMap(d => | ||
| d instanceof RandomItemGroupDrop | ||
| ? d.getDropItemIds() | ||
| : [d.getRawId()] | ||
| ) | ||
| ) | ||
| ); | ||
|
|
||
| const sortedIds = sortItems(allIds); | ||
|
|
||
| m += sortedIds.map(id => { | ||
| const item = getItemById(id)!; | ||
| return ` ${item.name} |cff00ff00[${item.classification}, Level ${item.level}]|r`; | ||
| }).join("\n"); |
There was a problem hiding this comment.
This makes all "items" within a set clamped into one big chunk. I like the previous one better.
Old
Now
| //A set that contains a list of specific items or multiple GroupDrops, or a mix of both | ||
|
|
||
| const ids = Array.from(new Set(set.itemDrops.flatMap(d => d.getDropItemIds()))); | ||
| const sortedIds = sortItems(ids); |
There was a problem hiding this comment.
Do sorting ones in findMapInitialCreepsWithDrops(). No need to sort them every time we show them.
| } else if (getItemById(itemOrGroupId) !== undefined) { | ||
| return [new SpecificItemDrop(itemOrGroupId)] | ||
| } else { | ||
| print(`Unknown item drop id "${itemOrGroupId}" for unit "${unit.name}" at (${unit.x}, ${unit.y}).`); |
There was a problem hiding this comment.
Comment instead. This might be useful in the future.
But im not even sure if this have to be disable, we could fix faster with this one enabled xD
There was a problem hiding this comment.
You can for example have a creep with
Item Set 1 (Total Chance: 100%)
- 100% - None
That would still trigger the debug message.
Debug message could still somewhat be acceptable for casual ladder games, but considering our triggers also get used for tournament games, I'd rather avoid them.
There was a problem hiding this comment.
All current maps do not print this msg.
If new map is added. Would it be better to have this msg, temporarily disable the feature/or msg, fix the bug in the feature or a map, then enable it back.
Or its better to hide all the bugs?
There was a problem hiding this comment.
I think it's a non-blocking issue, no? I would just print some warning messages when you run the script
There was a problem hiding this comment.
Not sure what you mean by
I would just print some warning messages when you run the script
But, if you think it's an issue - you can comment it out instead of disabling.
Ideally i would like for print() to be toggleable at runtime and enabled for dev builds.
Like a logging system. Then we could take somebody's replay, enable logging - and there would be prints/writes to disk when watching the replay - may help with debugging when issue happens for end users.
| if (drop instanceof RandomItemGroupDrop) { | ||
| if (!isTomeDrop(drop)) return false; | ||
| } else { | ||
| const item = getItemById(drop.getRawId()); | ||
| if (!item) return false; | ||
| if (item.classification !== ItemClass.Power_Up) return false; | ||
| if (item.level !== 1 && item.level !== 2) return false; | ||
| } |
There was a problem hiding this comment.
Move this to isTomeDrop (so it check for group tome drop and single item drop)
| if (!isTomeDrop(drop)) return false; | ||
| } else { | ||
| const item = getItemById(drop.getRawId()); | ||
| if (!item) return false; |
There was a problem hiding this comment.
Don't do those checks for items from drop sets - they are guaranteed to not be null anyway and it keeps it consistent with other code
| e = Effect.create("loot-indicator\\loot-indicator-generic.mdx", 0, 0)!; | ||
| } | ||
|
|
||
| e.scale = 1.5; |
There was a problem hiding this comment.
Check with @Kenshin on changes to indicator appearance. I think it looks better with this scale, but maybe too big? Like 1.3 - 1.4 maybe?
|
What does this mean?
|
Let's say you mistakenly added the same item twice within the same item set, for example Mantle +3, Slippers +3, and another Mantle +3. Then in the list of items (and the loot recap as well) you're going to see Mantle +3 listed only once. |
The command bar (and full drop table tooltip) should display exactly the items that were configured by the map maker. Also, don't complicate the code for the cases that do not exist in practice or don't cause any issues. |
|
Sorry, both. I haven't seen any content displayed by this indicator in the past two days of the competition or from the player's first perspective. I would like to confirm if it hasn't been implemented in the official environment map or for some reason. You have been improving the display effect, which is very exciting to both of you, but it seems that making the audience see it is more important? |
For discussion not relevant to the PR, use W3Champions discord server https://discord.gg/uJmQxG2 |
I'm honestly not sure about that. 1v1, 2v2 and 4v4 are mostly safe on the ladder because I look after them personally, but for anything else (FFA, ATR, any other non-ladder map we are asked to upload to Flo for whatever reason) it's totally unregulated and mapmakers are oftentimes able to freestyle as much as they like. Anyways not really a hill I'm willing to die on. It just seemed cleaner to me to avoid duplicates on the main list. |
|
I don't see value in displaying unique items right now (same goes for sorting). Maybe if we introduce drop percentages. |
|
@marcoabreu Can this be closed? If not, what all needs to be done? |
5 participants
