catch441 / Ultimate_Economy

GNU General Public License v3.0
15 stars 3 forks source link

Code for Issue #97 #99

Closed CowedOffACliff closed 4 years ago

CowedOffACliff commented 4 years ago

This is code adding the functionality in Issue #97. Issues with automatic merging are largely due to reformatting of files upstream after I started working on this combined with the tests that were added to test files I was required to modify. I've already made an attempt to merge the branches on my end so I'm hopeful that it spits out less confusing garbage for you. When I attempted it on my end there were 4 total files in conflict.

Changes: A new job type has been added which is triggered by two valid entities breeding together. Two commands have been added to operate the functionality: jobcenter job addBreedable and jobcenter job removeBreedable .

Small changes have also been made to the tab completion function that adds entities to the tab list for the regular addMob/removeMob commands. I used a test entity.isAlive() to eliminate a lot of useless things from the list that really don't belong like arrow entities and minecarts.

As a result of these changes the compile tests in two test files needed to be changed. In one file the new command types needed to be added in to the function which changed the total list size being tested for. The integers corresponding to the list sizes were raised by 2 in both cases. One of the cases tests for the total number of items returned in the tab completion of addMob/removeMob for which the integer value being tested for has been lowered from 104 to 67 to meet the change in functionality.

In UltimateEconomy.java the handleJobInfo() command has been modified. The new job type has been added and I have also modified the listing for Blocks to be consistent with other job types. The en_US resource bundle has been modified to have text corresponding to these additions and changes. Blocks are now prefixed with "Mine:" and the new breeding job type is prefixed with "Breed:". As a user I felt that the colon after the name of each descriptor combined with the prefixes created a more professional and consistent look for the data being displayed.

A static ArrayList now exists with a predefined list of all of the vanilla entities that can currently be bred (16 in total) which is used to cull entities from tab completion of the breedable commands. A TODO is marked here to add Striders and Hoglins to this list when Minecraft is officially updated to 1.16. Currently it's just positioned at the top of the JobSystemValidationHandler because its primary use was for a validation command. The list is also referenced when adding entities to the tab list for breedable commands. This data could probably be moved to a more elegant location but for now it's functional.

Currently Missing: Things currently missing from this are the addition of new unit tests corresponding to the newly added commands. The tests required will be nearly identical to those for addMob/removeMob commands but probably with a different integer value (16 in 1.15, 18 in 1.16) being tested for the tab completion list. Resource bundles have also not been modified for any language but my native English.

CowedOffACliff commented 4 years ago

Looks like the only remaining conflict is in the validation handler which should just be additional code with no other changes.

CowedOffACliff commented 4 years ago

Fixed conflicts with new code functions. Added code to fix issue #98. Removed source files for com.mojang.authlib from the project because they're duplicates of the one provided by the Spigot artifact and they just confuse the build. Marked Spigot artifact as 'provided' instead of 'test' so that it builds properly.

Give 'er a whirl, chief.

CowedOffACliff commented 4 years ago

Fixed another NullPointer crash in the plugin loading process.

CowedOffACliff commented 4 years ago

Shop UI is totally unresponsive. Commands can still edit the contents of the shops but attempting to purchase/sell items or trigger any click event fails completely. For some reason the item stack being passed via the InventoryClickEvent is null even if you are clicking a valid item.

catch441 commented 4 years ago

the actual version inside the dev branch has some commented out code due refactoring, so some functionality is not working right now as you saw.

CowedOffACliff commented 4 years ago

the actual version inside the dev branch has some commented out code due refactoring, so some functionality is not working right now as you saw.

After messing with it for a while I realized that was the missing code and uncommented it just to see how broken the changes had made it. The answer was "hornet's nest." Without knowing all of changes that one is a project for you to finish. XD

CowedOffACliff commented 4 years ago

Will work on getting tests added equivalent to tests for similar functions. Will copy/paste text from addMob and removeMob commands into the various language files so that they will at least be functional for users until real translations can be donated. The added line for the "Mine:" text is a bit of a problem to stew on because language is complicated but after looking at translations of the official Minecraft wiki it's apparent that for most languages poaching a translation of "remove" or "destroy" will probably suffice.

I'm just a guy not a professional so my implementations of things will not always be the best way of doing it. If you ever have a question about something I will usually be able to give a specific explanation of the exact reason for a piece of code and what I encountered that made me determine a need for it.

As far as actually working on the project I have a lot to do and not a crazy amount of free time, but I am running a server with this plugin and I see its potential. I've got some motivation for improving it and seeing if in time I can work out how to implement some of the more difficult things I'd like to see added or upgraded. Personally I'm aiming to get a stable feature complete version of the development branch that I can run in the live environment so that I can get features directly to the server for tinkering.

CowedOffACliff commented 4 years ago

Basically waiting for the Paper build of 1.16 to come out before I get motivated to finish this and go through another merge.

catch441 commented 4 years ago

merged