Conversation
Machine-Maker
force-pushed
the
feature/better-stats-api
branch
2 times, most recently
from
1544ca8 to
d0244c7
Compare
Machine-Maker
moved this from Awaiting review
to Awaiting final testing
in Paper PR Queue
| + | ||
| + // Paper start | ||
| + if (io.papermc.paper.event.player.PlayerRequestStatisticsEvent.getHandlerList().getRegisteredListeners().length > 0) { | ||
| + io.papermc.paper.event.player.PlayerRequestStatisticsEvent statEvent = new io.papermc.paper.event.player.PlayerRequestStatisticsEvent( | ||
| + player.getBukkitEntity(), | ||
| + map.object2IntEntrySet() | ||
| + .stream() | ||
| + .collect(Object2IntOpenHashMap::new, (map1, entry) -> map1.put(io.papermc.paper.statistic.PaperStatistics.getPaperStatistic(entry.getKey()), entry.getIntValue()), Object2IntOpenHashMap::putAll) | ||
| + ); | ||
| + if (!statEvent.callEvent()) { | ||
| + return; | ||
| + } | ||
| + map = statEvent.getStatisticMap().object2IntEntrySet().stream().collect(Object2IntOpenHashMap::new, (map1, entry) -> map1.put(io.papermc.paper.statistic.PaperStatistics.getNMSStatistic(entry.getKey()), entry.getIntValue()), Object2IntOpenHashMap::putAll); | ||
| + } |
There was a problem hiding this comment.
I am not quite sure about canceling this? As this will cause the menu to just reveal a blank screen which may be very confusing. At least, I think this should be documented.
There was a problem hiding this comment.
In my Stats request PR I answered with an empty map because the client would show a loading screen until it got an update if the stats weren't cached before
(Not 100% sure if that is vanilla behavior since I have a few mods installed that might change that behavior, but just in case)
Maybe the same documentation I had could be applied?
Machine-Maker
force-pushed
the
feature/better-stats-api
branch
from
80a4b7f to
201d4e4
Compare
Lulu13022002
force-pushed
the
feature/better-stats-api
branch
2 times, most recently
from
9a5d7dd to
c065539
Compare
| * | ||
| * @return the stat owner | ||
| */ | ||
| S owner(); |
There was a problem hiding this comment.
I don't fully understand the renaming to #owner
Wouldn't #type make more sense?
Owner sounds like the owning player of the statistic
There was a problem hiding this comment.
I don't really know what to call it. Nothing seems good to me. type feels wrong, cause its not StatisticType.
There was a problem hiding this comment.
What about those options: #statistic, #kind, #metric?
But in general I thought #value was a good fit.
There was a problem hiding this comment.
value feels like it should be the numeric value.
Lulu13022002
force-pushed
the
feature/better-stats-api
branch
2 times, most recently
from
fed3d15 to
ebd1340
Compare
Machine-Maker
force-pushed
the
feature/better-stats-api
branch
2 times, most recently
from
cb8b037 to
89ab2a1
Compare
| * | ||
| * @return the mutable statistic map | ||
| */ | ||
| public Object2IntMap<Statistic<?>> getStatisticMap() { |
There was a problem hiding this comment.
Only exposing a regular Map<Statistic<?>, Integer> would seem better for future proofing, Object2IntMap extends that so no extra conversion is needed.
There was a problem hiding this comment.
Not sure then plugins can't get the primitive without (un)boxing or casting the map.
There was a problem hiding this comment.
In some cases it would still help having it backed by the fu map, but not really here
Lulu13022002
force-pushed
the
feature/better-stats-api
branch
from
9cbdabe to
b9342e1
Compare
Lulu13022002
force-pushed
the
feature/better-stats-api
branch
from
9623d04 to
5cb864b
Compare
6 participants
Replaces #11834