garbagemule / MobArena

MobArena plugin for Minecraft
GNU General Public License v3.0
195 stars 151 forks source link

add reward per kill, add aggregator for monetary rewards to prevent p… #754

Closed JL-III closed 7 months ago

JL-III commented 1 year ago

…layer spam at end of game, add player notification for kills per wave

Summary

Feature to add per kill rewards to encourage players to participate.

Aggregate monetary reward announced when leaving the arena as apposed to 25+ lines of monetary reward messages.

Player shown their kills per round and per game (at end of game)

Problem

QOL improvements and more flexibility for mob arena rewards.

Solution

Not really more to say on the feature additions.

Action

If you could review the per kill rewards portion, I had some trouble understanding the thing parser and things concept. Thanks for taking the time to look at this! I love all the work you have already done!

garbagemule commented 1 year ago

Thanks so much for taking the time to take a stab at implementing something like this!

It seems like this is sorta kinda what's hinted at in the not-very-complete feature request #734, so I'm just referencing it here so we can close it if this PR ends up getting merged.

I have a couple of comments about the implementation itself, and I'll follow up with a discussion about how we might proceed.

Alright, so the feature itself is a great idea - giving server owners a way to further customize how rewards are earned during a session. It's also long overdue, I think. The reason the rewards section has a waves subsection is exactly because I anticipated different reward earning contexts, such as kills, but it was just never implemented. While I do appreciate the effort in this PR, I think the idea itself needs a lot more fleshing out. It's definitely not the best version of itself right now. We can do better!

First of all, I think it would be good to explore the dimensions of the feature. From the configuration details of this PR, it's clear that "a kill is a kill" is not how we want this to work. We can look straight to the waves subsection for inspiration: after and every. We could very well use kill count as a dimension for per-kill rewards. For instance, every 3 kills, you get an iron ingot, and after 20 kills, you get a gold ingot. Kill count is the most natural dimension for kills for the same reason wave number is the most natural dimension to work with for waves.

In this PR, we don't use that dimension, however. We use a fixed value for the kill count dimension, and instead introduce wave number as a dimension. This is interesting for two reasons: it makes it possible to change the per-kill rewards as the arena session progresses, and it creates a hard dependency on the wave number.

That last bit is crucial; if we create a hard dependency on wave numbers, it makes the Sessions Rework (which may introduce ways to configure arenas without waves per se) even harder than it already is. To make things not-so-icky, we need a layer of indirection. That layer might already be available to us via the Formulas API. It already contains a current-wave variable that could be used for some sort of branching/interval logic.

So as a bit of a TL;DR in terms of how we move on from here:

I hope all of that makes sense. Thanks again for the effort, and please feel free to hop on Discord if you'd like to have a less "formal" chat about all of this.

JL-III commented 1 year ago

Thanks for taking a serious look at this. I had to come back and look at this twice to wrap my head around everything you said.

I do see all the points you are making and in retrospect this PR was rather oblivious to the entirety of the codebase. I will have to take a deeper dive into the architecture to see how the kill rewards feature could be implemented in a way that remains consistent with the rest of the plugin.

I didn't even consider the current-wave variable you mentioned. wow lol

I'll spend some time on this, scope down the amount of changes in this PR to just the Per Kill (or After, Every Kill) Reward concept, and try to come back with something that works more with the APIs that are in place currently.

garbagemule commented 7 months ago

Closing this to clear out stale PRs, but please feel free to reopen if you revisit it 😃 I think per-kill rewards are long overdue, and they should be possible with the current structure of the config-file, although it does probably require a bit of tinkering with a few different portions of the codebase, including the gnarly MAUtils.