Skip to content

Changes & Fixes #1

@ShawnClake

Description

@ShawnClake

These are a list of changes and fixes which I would like applied to the code base.

Changes:

  • Rename EssentialsChat, EssentialsCheat, etc. classes to just Chat, Cheat, etc. The 'Essentials' part is implied by the namespace and thus unnecessary.
  • Change the access modifier for the Chat, Cheat, etc. variables in RRPGEssentials to be public static currently, nothing is specified.
  • All the overridden functions should not be returning null. Instead, have them all return new ArrayList<>() Why cause a null ptr exception if we dont need to.
  • The overview string in RRPGEssentials should be improved. Currently it is unclear as there's no such thing as an 'essentials server'
  • Remove any instance of a comment that contains 'Created by blah blah' at the top of the classes you create.
  • Any command handler you've boiler plated, but haven't implemented, just add a single line to it that tells the player that the command isn't ready yet. For example: interactable.sendMessage("This command is coming soon"); Additionally, ensure they at least return a blank string for the command format. Why cause a null ptr if we dont have to.
  • I'm not sure how you're trying to use player data... But it won't work the way you're trying. Change all your usages of player data to look more like this: RRPGCore.players.getPlayerData(bukkitPlayer, TopSpeed.class).someFunctionOnYourPlayerDataClass;
  • You have a function called 'getLocation' on your PlayerHomes class. That is an ambiguous name, change the name of the function to 'getHome'
  • In your homes class, just print out each home they have on a separate line. No need to waste CPU cycles doing fancy concatenation.
  • In all of the command handlers where you have the ArrayList arguments variable named 'arrayList' change that to something meaningful like 'args' or 'arguments'. The name 'arrayList' is meaningless and makes the code hard to read and maintain.
  • Remove the inner class 'Home' from your 'PlayerHomes' class. Just make a brand new class called 'Home' and use that. Inner classes make code hard to read and far less maintainable.
  • Any class variable you ever use must have an access modifier. In general, these should always be 'private'. Never make a class variable without an access modifier. That is a horribly bad practice which can cause unpredictable behavior.
  • In 'PlayerHomes' function 'listHomes' you create and return a string array. Don't do this. Create and return a string ArrayList. Arrays are awkward to work with in Java, and I would rather take the memory hit from using an ArrayList than the CPU cycle hit from using work arounds on arrays.
  • Add a player data to represent when a player is AFK. This allows for additional functionality in the future, such as making them untargettable while AFK.
  • Add permissions to the other command handler classes which dont have them. IE: home
  • This class is your friend https://github.com/OpenRubiconProject/rrpg-core/blob/master/src/com/openrubicon/core/api/server/players/Players.java start using it instead of manually converting UUID's to players manually.
  • I don't think the TP mechanism should be using player data. Player data is meant for tracking changing data, or for long term data. It is not for short lived quick data like teleport requests. Create a separate class 'Teleport Requests' with a system for adding, removing, and acting upon teleport requests as they come in and are used.
  • Remove the god command, this is already implemented in the combat module.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions