Tale of Kingdoms: A new Conquest

Tale of Kingdoms: A new Conquest

49.7k Downloads

Implemented suggestions from #42, #43, and #48 - [merged]

SamB440 opened this issue ยท 49 comments

commented

In GitLab by @JordanPlayz158 on May 14, 2021, 21:13

Merges master -> master

As described in the title, I have implemented the suggestions described/specified in issues #42, #43, and #48.

commented

No star imports

commented

I am not sure about having a different entity, but if it's absolutely neccesary, could it be GUILDMASTER_DEFENDER or something more descriptive than just clone?

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

No star imports

commented

Should have private modifier

commented

Why REPLACE_EXISTING? do we not want this configurable? this would reset it?

commented

No star imports

commented

As we have a separate entity, I'm pretty sure you can get rid of this method and just put it in the init goals.

commented
commented

In GitLab by @JordanPlayz158 on May 15, 2021, 14:33

added 2 commits

  • be38028 - Revert "Changes: - Let the ide optimize the imports of all of our code...
  • b18ba38 - Re-commited changes from the revert that were accepted.

Compare with previous version

commented
commented
commented
commented
commented
commented

In GitLab by @JordanPlayz158 on May 15, 2021, 14:40

added 1 commit

  • e6b86dc - Added private access modifier to the worthinessJson JsonObject.

Compare with previous version

commented

In GitLab by @JordanPlayz158 on May 15, 2021, 14:54

Commented on src/main/java/com/convallyria/taleofkingdoms/client/entity/render/RenderSetup.java line 23

Ok it will be renamed to GUILDMASTER_DEFENDER and i'm not sure if it's absolutely neccesary but it was the way that made the most sense in my head but in the future, if we feel it is unnecessary we can change it.

commented

In GitLab by @JordanPlayz158 on May 15, 2021, 14:54

Commented on src/main/java/com/convallyria/taleofkingdoms/common/world/ConquestInstance.java line 128

Done, will be committed soon after testing that everything still works.

commented
commented

In GitLab by @JordanPlayz158 on May 15, 2021, 15:07

added 1 commit

  • 236a93f - Addressed all suggestions in pull request !32:

Compare with previous version

commented

In GitLab by @JordanPlayz158 on May 15, 2021, 15:23

Commented on src/main/java/com/convallyria/taleofkingdoms/common/listener/CoinListener.java line 129

Nono, line 128 verifies that the file does not already exist, I'm not sure why but back when I tried copying files, this is the only way I could get it to work but it does work fine, it doesn't replace or reset the config, it will only copy the file to that location if the file doesn't already exist.

commented

resolved all threads

commented

approved this merge request

commented

mentioned in commit 690e50e