Version 1.0 CMI Extention #153
Conversation
…b.com/plan-player-analytics/Extension-EssentialsX thus its structure is a also near identical.
AuroraLS3
left a comment
AuroraLS3
left a comment
There was a problem hiding this comment.
Looks good, I put some comments.
I would suggest against creating multiple Extensions for one plugin and using @TabInfo and @Tab annotations instead - Here's an example https://github.com/plan-player-analytics/Extension-LuckPerms/blob/master/src/main/java/net/playeranalytics/extension/luckperms/LuckPermsExtension.java
src/main/java/net/playeranalytics/extension/CMI/CMIEcoEventListener.java
Outdated
Show resolved
Hide resolved
src/main/java/net/playeranalytics/extension/CMI/CMIExtensionFactory.java
Outdated
Show resolved
Hide resolved
|
With this commit 7923936, this now uses only one Extension and Tabs for the diffrent types of data. This now should be ready to be merged This was implemented this way, as the Extension of EssentialsX also did it this way. If there aren't any reasons why I shouldn't I will also make a PR for EssentialsX with those changes to be coherent |
| */ | ||
|
|
||
| @EventHandler(priority = EventPriority.MONITOR) | ||
| public void onBalanceChange(CMIUserBalanceChangeEvent event) { |
There was a problem hiding this comment.
This event has the potential to be fired 100s of times per second when combined with something like Jobs that pays out per block mined. Considering that I will add a time based limiter on this updating the Plan data, since currently each call to updatePlayerData would fire ~24 transactions in Plan
AuroraLS3
left a comment
AuroraLS3
left a comment
There was a problem hiding this comment.
Approved. I will close this PR instead of merging in order to fork the repository instead.
If I remember correctly the Essentials Eco portion of the extension is disabled, I might misremember though. |
Looks like CMI requires spigot artifact built by BuildTools which is not deployed in a maven repository anywhere, making jitpack pointless. |
|
CMI API also targets Java 21 so this will take some more effort to get merged. Changes in the other PR plan-player-analytics/Plan#3815 may be required in order to retain Java 11 support. |
2 participants
This is the first Version of the CMI Extension as requested in ISSUE #3735 it is, for now, a feature copy of Extention-EssentialsX
In theory the CMI-API should be available threw Jitpack though I think the author of the API made a mistake configuring Jitpack, I wasn't able to get it threw this repository and build it localy in order to make it available, maybe someone else can have a look at this, maybe I missunderstood something.