[MEGA THREAD] Get Your Code Reviewed or Review Others | SpigotMC | Page 3

round fossil · 2023-10-21T02:06:26.287Z

I mean create as many as you have to create just dont over create

round fossil Oct 21, 2023, 2:06 AM

#

I mean create as many as you have to create shrug just dont over create

rough hound Oct 23, 2023, 7:51 AM

#

Hey can I get feedback on my first plugin https://github.com/CMSombre/RiddlesFriends

jagged rock Oct 23, 2023, 7:53 AM

#

404 - Private repo probably

#

Missing readme file, lack of documentation
You shouldn't post your .idea folder
RiddlesFriendMain is a weird class name for your plugin entry point, I'd call it FriendsPlugin or RiddleFriendsPlugin
Your plugin class isn't a data class, you shouldn't be tracking friend requests and friend data there
You should split your command logic into a separate command class
You shouldn't hardcode your messages
You should use the player UUID rather than the player name in your maps and operations
You should make your sounds configurable
On your FriendRequestManager, don't expose a mutable list, rather return an immutable copy, or an immutable empty list (ImmutableList.copyOf, Collections.EMPTY_LIST)
You don't use the FriendData or FriendRequestManager classes
I'd split your IO in a separate class

pine grail Oct 23, 2023, 7:59 AM

#

Summed it up perfectly

hexed agate Oct 23, 2023, 8:00 AM

#

posting .idea is fine for a subset of the files in it, such as just a few IDE settings, but most of the files shouldn't be pushed true

pine grail Oct 23, 2023, 8:00 AM

#

missed most of that (just generally skimmed trough)

jagged rock Oct 23, 2023, 8:01 AM

#

For commands, you should further work on a subcommand system, or try out a command framework, but also re-use variables like args[0]

pine grail Oct 23, 2023, 8:01 AM

#

switch(args[0].toLowerCase(Locale.ROOT)) {
  case "a" -> a();
  case "b" -> b();
  case "c" -> c();
}

reposting here from #general, you can also use switch statements

jagged rock Oct 23, 2023, 8:01 AM

#

Good points for a first plugin:

Decent variable naming
Splitting stuff into methods
Guard clauses, little nesting
No static abuse
Good usage of try-with-resources

#

I'll remove a point for using JSON over the industry standard YML

pine grail Oct 23, 2023, 8:03 AM

#

jagged rock - Missing readme file, lack of documentation - You shouldn't post your .idea fol...

Adding onto this really quickly

Don't check for things like command name
If you are above java 16, try to use records for immutable data
Make variables final where they can be made final
Use packages if you need to have multiple classes of a category (like sub-command classes)

jagged rock Oct 23, 2023, 8:04 AM

#

I'm old-fashioned personally and don't like records but eh

pine grail Oct 23, 2023, 8:04 AM

#

They are useful for data that won't mutate

#

makes it much easier to not care about boilerplate for immutable dataclasses

jagged rock Oct 23, 2023, 8:05 AM

#

lombok clown_2

pine grail Oct 23, 2023, 8:05 AM

#

no

#

Heyo I'm in the mood for some flames, be warned that this is just "eh whenever I can I'll add some code", no real thought put into it for now -> https://github.com/IkeVoodoo/spigot-core

GitHub

GitHub - IkeVoodoo/spigot-core: A set of small core utils for spigo...

A set of small core utils for spigot, spigot-core! - GitHub - IkeVoodoo/spigot-core: A set of small core utils for spigot, spigot-core!

hexed agate Oct 23, 2023, 8:08 AM

#

that is a lot of fucking modules

pine grail Oct 23, 2023, 8:08 AM

#

pick one

#

they have all the same quality lmao

hexed agate Oct 23, 2023, 8:08 AM

#

ill pick a random one and look at it but im not looking at them all

pine grail Oct 23, 2023, 8:08 AM

#

yeah no

#

do that

#

like pick just one

#

98x java files, 6,652 LOC

hexed agate Oct 23, 2023, 8:10 AM

#

looks fine to me after just some light looking but i see some system.out.println in there which earns you a kekpoint

pine grail Oct 23, 2023, 8:10 AM

#

yeah quick debugging

#

which module

hexed agate Oct 23, 2023, 8:10 AM

#

https://github.com/IkeVoodoo/spigot-core/blob/master/spigot-core-entities/src/main/java/me/ikevoodoo/spigotcore/entities/listeners/EntityLoadListener.java

pine grail Oct 23, 2023, 8:10 AM

#

ah yeah I am working on that stuff still

#

It doesn't want to load the markers lmao

hexed agate Oct 23, 2023, 8:11 AM

#

well looks okay to me but i didn't read it like a book just skimmed around

pine grail Oct 23, 2023, 8:11 AM

#

yeah

hexed agate Oct 23, 2023, 8:11 AM

#

i'll look deeper later maybe if i have time

pine grail Oct 23, 2023, 8:11 AM

#

ye sleep

jagged rock Oct 23, 2023, 8:14 AM

#

I'll remove points for not making your own classes for built-in annotation processors on your config system

#

and rather using lambdas

pine grail Oct 23, 2023, 8:14 AM

#

wdym?

#

ah

jagged rock Oct 23, 2023, 8:14 AM

#

these

pine grail Oct 23, 2023, 8:14 AM

#

Yeah

#

They where one liners and I forgor to move them

#

Currently at -1 points

jagged rock Oct 23, 2023, 8:15 AM

#

Not a huge fan of reflections so I won't look into this further

pine grail Oct 23, 2023, 8:16 AM

#

Yep, I realized this system is completely flawed

jagged rock Oct 23, 2023, 8:16 AM

#

Alright I really hate your debugging module

pine grail Oct 23, 2023, 8:16 AM

#

Oh yeah that one

#

wanna know why I made it

jagged rock Oct 23, 2023, 8:16 AM

#

no

pine grail Oct 23, 2023, 8:16 AM

#

nice

#

Points: -2

jagged rock Oct 23, 2023, 8:17 AM

#

isn't this technically unsafe

pine grail Oct 23, 2023, 8:17 AM

#

where is that

jagged rock Oct 23, 2023, 8:17 AM

#

debug reporter

#

upload to hastebin

pine grail Oct 23, 2023, 8:17 AM

#

ah yeah

#

I hacked that class together because I needed to get smt out fast

jagged rock Oct 23, 2023, 8:17 AM

#

like if it just returns a 404

#

or something

pine grail Oct 23, 2023, 8:18 AM

#

nope

jagged rock Oct 23, 2023, 8:18 AM

#

invalid urls yada yada

pine grail Oct 23, 2023, 8:18 AM

#

that's checked above

#

jagged rock Oct 23, 2023, 8:18 AM

#

ok ok

#

wtf is this

pine grail Oct 23, 2023, 8:19 AM

#

It returns the version of spigot core being used (it's shared across all modules)

jagged rock Oct 23, 2023, 8:20 AM

#

alright spawned entities

#

I hate this already

pine grail Oct 23, 2023, 8:20 AM

#

The entity module?

jagged rock Oct 23, 2023, 8:20 AM

#

yeah

#

I'd rename Entity -> AbstractEntity to avoid conflict with bukkit's entity

pine grail Oct 23, 2023, 8:21 AM

#

fair point

#

I should also rename Item

jagged rock Oct 23, 2023, 8:21 AM

#

I don't like how often you call getProvidingPlugin so I'd probably just have a variable somewhere and reuse it across the entire module

#

this could be switched with a more functional approach

#

For example, a ClickAction enum

pine grail Oct 23, 2023, 8:22 AM

#

Hm?

jagged rock Oct 23, 2023, 8:22 AM

#

and a registerClickAction(ClickType.RIGHT_CLICK, (context) -> {});

#

type deal

pine grail Oct 23, 2023, 8:22 AM

#

Oh yeah

#

that would work

jagged rock Oct 23, 2023, 8:22 AM

#

this never makes it clear if you're spawning the entity parts as well

#

Or what

#

I might be tripping

pine grail Oct 23, 2023, 8:23 AM

#

No no you're right

jagged rock Oct 23, 2023, 8:23 AM

#

bad naming

#

This system could be adapted for packet entities as well, so this could be named BukkitArmorstandEntityPart

pine grail Oct 23, 2023, 8:24 AM

#

Yeah should've been ItemEntityPart

jagged rock Oct 23, 2023, 8:24 AM

#

or something

pine grail Oct 23, 2023, 8:24 AM

#

jagged rock This system could be adapted for packet entities as well, so this could be named...

Oh fair

jagged rock Oct 23, 2023, 8:24 AM

#

And then a PacketArmorstandEntityPart in the future

#

So you could then do a BukkitDisplayEntityPart

#

etc etc

#

you get the idea

pine grail Oct 23, 2023, 8:25 AM

#

Yep I agree

jagged rock Oct 23, 2023, 8:26 AM

#

ItemSelector is a bit nifty

pine grail Oct 23, 2023, 8:26 AM

#

how so

jagged rock Oct 23, 2023, 8:26 AM

#

Not a fan of how you're hardcoding next and previous to be final

pine grail Oct 23, 2023, 8:27 AM

#

yeah those are defaults

#

they can be overridden

jagged rock Oct 23, 2023, 8:27 AM

#

For example, on some menus I've made, if you were in the first page you don't have a previous button and it is another item

pine grail Oct 23, 2023, 8:28 AM

#

nvm you can't set them

#

got the wrong class lol

#

oh no you def can, but yeah it should be like an item stack provider

#

and not a final item

jagged rock Oct 23, 2023, 8:28 AM

#

I'd rather make a MenuElement interface with impls such as Button (itemstack / supplier), Switch (multiple items, each has its element) etc

#

and nuke the itemselector class in favor of switch

pine grail Oct 23, 2023, 8:29 AM

#

Oh yeah

jagged rock Oct 23, 2023, 8:30 AM

#

SelectionConverter could be useful for the multiswitch but eh

#

I'd only give menu context to my elements on the getItem function, maybe..

#

ItemSelector shouldn't be responsible for managing its own screen

#

unless your Screen is my MenuLayer

#

even then, icky

pine grail Oct 23, 2023, 8:32 AM

#

👍

jagged rock Oct 23, 2023, 8:33 AM

#

ScreenPaginator in every screen? fuck no

pine grail Oct 23, 2023, 8:33 AM

#

what

jagged rock Oct 23, 2023, 8:33 AM

#

Just make a layer system with a SimpleLayer and a PaginableLayer

willow bone Oct 23, 2023, 8:40 AM

#

Code review? https://github.com/Sweattypalms/skyblock-remake

GitHub

GitHub - Sweattypalms/skyblock-remake: Hypixel skyblock remake

Hypixel skyblock remake. Contribute to Sweattypalms/skyblock-remake development by creating an account on GitHub.

jagged rock Oct 23, 2023, 8:42 AM

#

this is too cursed for me

#

and has nothing to do with skyblock

willow bone Oct 23, 2023, 8:42 AM

#

Wdym

#

🗿

sly jackal Oct 23, 2023, 9:29 AM

#

apart from the name, there is nothing in here to make you believe its skyblock

#

could be for any rpg like plugin

#

I think thats what illusion means

#

im not really a fan of the static lists you have for mobs and items, etc

sly jackal Oct 23, 2023, 10:02 AM

#

I would just use DI if you can

#

you wont be needing those lists in a ton of places I reckon

round fossil Oct 23, 2023, 12:30 PM

#

Don’t use sysout to log stuff

night knoll Oct 23, 2023, 5:15 PM

#

Custom async B)

#

After 69420 jvm crashes...

cobalt trellis Oct 23, 2023, 5:19 PM

#

JVM crashes? What the hell...

#

Ah probably stuff in native space

opaque plover Oct 23, 2023, 5:20 PM

#

night knoll Custom async B\)

Lua spigot coding??

night knoll Oct 23, 2023, 5:20 PM

#

Yup

opaque plover Oct 23, 2023, 5:21 PM

#

I've never seen something so cursed that I also loved so much

surreal peak Oct 23, 2023, 5:21 PM

#

god dammit is that lua?

night knoll Oct 23, 2023, 5:21 PM

#

ofc

opaque plover Oct 23, 2023, 5:21 PM

#

surreal peak god dammit is that lua?

I was literally looking up your name to see when your last message was because I thought I haven't seen you in a while and you appeared lmao

rough hound Oct 23, 2023, 5:53 PM

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java

Can someone check my main class again, I did more methods and switches this time around

surreal peak Oct 23, 2023, 5:58 PM

#

return friendData.containsKey(player) && friendData.get(player).contains(friend); ee two lookups

pine grail Oct 23, 2023, 6:04 PM

#

surreal peak `return friendData.containsKey(player) && friendData.get(player).contains(friend...

Translation:

FriendData friendData = friendData.get(player.getUniqueId()); // Use UUID instead of Player, it's safer

// "friendData != null" is the same as containsKey
return friendData != null && friendData.contains(friend.getUniqueId()); // Again use UUID instead of Player

cobalt trellis Oct 23, 2023, 6:04 PM

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L23-L25 these fields could probably be final (the initialization would then happen in the constructor or init block)

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L51 This could also be for Command blocks, FYI

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L47 and https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L74 errors or warnings are usually colored red (same with pretty much every other line)

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L82-L84 mkdir already has that check embedded, iirc

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L139-L140 may be vulnerable to attacks due to the complexity of #contains in a List being O(n) and the fact that you allow invalid entries and never purge entries (even across restarts!).

cobalt trellis Oct 23, 2023, 6:05 PM

#

pine grail Translation: ```java FriendData friendData = friendData.get(player.getUniqueId(...

They do not use Player - don't worry

pine grail Oct 23, 2023, 6:05 PM

#

cobalt trellis They do not use `Player` - don't worry

ah alright

cobalt trellis Oct 23, 2023, 6:05 PM

#

However it is still better to use UUIDs over Strings.

surreal peak Oct 23, 2023, 6:06 PM

#

pine grail Translation: ```java FriendData friendData = friendData.get(player.getUniqueId(...

friendDate.getOrDefault(x, Collections.emptyList()).contains(friend) 😉

cobalt trellis Oct 23, 2023, 6:06 PM

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L128 returning an empty list (via Collections.emptyList()) may be better here

surreal peak Oct 23, 2023, 6:07 PM

#

just check for null, youre not reusing it

cobalt trellis Oct 23, 2023, 6:08 PM

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L130 long friend lists aren't broken up in multiple messages and the format of ArrayList#toString isn't actually that user-friendly to be worth using.

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L148 this check has already been performed on line 139, you might want to not duplicate it.

#

Furthermore you may want to replace all mentions of /friend with the respective alias that was used to invoke the command, just in case the server owner defined an alias via the commands.yml file or even overwrote the command (which can be necessary in case of plugin conflicts)

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L101 this is a very ... strange line. Are you sure that makes sense?

Additionally, to some degree I'd recommend using NIO instead of java.io.File but as bukkit is already using java.io.File there is no notable difference

cobalt trellis Oct 23, 2023, 6:15 PM

#

surreal peak just check for null, youre not reusing it

That isn't actually viable as you can remove friends but in that case the list of friends may be empty.

#

Well you could still have a null check, but the isEmpty check is still required

#

https://github.com/CMSombre/RiddlesFriends/blob/master/src/main/java/me/cmriddles/riddlesfriends/RiddlesFriendMain.java#L242 you can always return true here. Instead, isFriendRequestPending is replaced by removeFriendRequest (at line 240)

rough hound Oct 24, 2023, 12:49 AM

#

Hey guys, I updated my plugin, can I get feedback
https://github.com/CMSombre/RiddlesFriends

round fossil Oct 24, 2023, 12:52 AM

#

3 classes isnt a lot

rough hound Oct 24, 2023, 12:52 AM

#

huh?

round fossil Oct 24, 2023, 12:53 AM

#

To what extent do you want feedback?

rough hound Oct 24, 2023, 12:53 AM

#

at my code, cuz I'm new to programming plugins

round fossil Oct 24, 2023, 12:53 AM

#

Is there anything in particular you’d like me to look at?

rough hound Oct 24, 2023, 12:54 AM

#

just the overall structure if theres anything I can improve on, maybe my DatabaseHandler and FriendDataCache, I have 34 warnings and idk why

round fossil Oct 24, 2023, 12:54 AM

#

Perhaps start by going through then lol

#

Your DataBaseHandler isn’t a handler

#

Its called Database not DataBase btw

#

You can turn it into a record btw

#

Your FriendDataCache isn’t a cache

#

You just directly talk to mysql everytime

rough hound Oct 24, 2023, 12:57 AM

#

huh? isnt that the point of the cache, to relay to the database

#

and the handler connects to the database while the cache relays info the database?

round fossil Oct 24, 2023, 12:57 AM

#

Nah, I mean when we talk about cached data, we mean data that’s stored somewhere where its fast and easy to access

#

Database calls take time, a lot of time and are heavily io intensive

#

So if it were a cache, it would be that you store the data accessed from mysql in some sort of Map or List, or other data structure during the runtime of the server

#

In that way its accessible and fast to access

rough hound Oct 24, 2023, 12:59 AM

#

ahh, so my class names are irrelevant then to what I coded

round fossil Oct 24, 2023, 1:00 AM

#

Ugh well

#

I’d rename it to MySqlStorage

sleek shard Oct 24, 2023, 1:00 AM

#

hi concLUBE how are you

round fossil Oct 24, 2023, 1:00 AM

#

And DatabaseCredentials or sth

round fossil Oct 24, 2023, 1:00 AM

#

sleek shard hi concLUBE how are you

Im finw hbu

sleek shard Oct 24, 2023, 1:00 AM

#

im good just chillin

round fossil Oct 24, 2023, 1:00 AM

#

nicee

rough hound Oct 24, 2023, 1:01 AM

#

understood!

#

Overall how is my code?

round fossil Oct 24, 2023, 1:01 AM

#

Its decent

#

I mean obviously you should probably learn the object oriented paradigm in java if you now code in java

#

That means Object Orientation, read Effective Java, learn SOLID, learn a bit about Functional Programming in Object Oriented Fashion, learn a bit about the Data Oriented Programming paradigm, and learn how to deal with concurrency in Java

#

Now that’s a lot of stuff

#

So don’t go on and learn everything I just mentioned

#

But pick something, and try to implement it in your code (:

rough hound Oct 24, 2023, 1:05 AM

#

thank you for the feedback

lunar vale Oct 24, 2023, 5:14 AM

#

the class extending javaplugin should be NamePlugin

cobalt trellis Oct 24, 2023, 10:11 AM

#

Meh not necessarily

willow bone Oct 24, 2023, 11:56 AM

#

lunar vale the class extending javaplugin should be NamePlugin

Thats kinda cringe!!

round fossil Oct 24, 2023, 1:09 PM

#

No that’s actually a good point @lunar vale has

jagged rock Oct 24, 2023, 1:09 PM

#

Yeah I do it all the time

#

better than calling it "Main"

round fossil Oct 24, 2023, 1:10 PM

#

Since its a derivative of JavaPlugin, where the common term is Plugin (it even appears in the superclasses of JavaPlugin), similarly people do this with different types of exceptions, Exception -> RuntimeException -> IllegalArgumentException etc

jagged rock Oct 24, 2023, 1:11 PM

#

I'll also add that it should be the only class in that specific package

#

or have its own package

round fossil Oct 24, 2023, 1:11 PM

#

Good rule of thumb also

jagged rock Oct 24, 2023, 1:11 PM

#

For example

#

Don't name your listeners package "events", that implies that's where the custom events are
Don't bury important classes deep into packages, they should be intuitive to find if I'm just skimming through the folders

round fossil Oct 24, 2023, 1:20 PM

#

There is also a good name convention that packages are to be named singular

#

Just look at the std lib

cobalt trellis Oct 24, 2023, 1:27 PM

#

My main classes are usually called Name

#

Except for Presence where it is called PresenceBukkit, even though it doesn't run on bukkit.

sly jackal Oct 24, 2023, 5:40 PM

#

round fossil There is also a good name convention that packages are to be named singular

if you had a package to store settings related objects, do you call it setting or settings?

round fossil Oct 24, 2023, 5:42 PM

#

That’s extremely abstract

pine grail Oct 24, 2023, 5:42 PM

#

settings

#

ofc

round fossil Oct 24, 2023, 5:43 PM

#

But for instance if you have a package of utilities you call the package util, and for commands you call it command etc

night knoll Oct 24, 2023, 11:28 PM

#

night knoll Custom async B\)

I know it has been 2 days ago, but...
c# for spigot when?

#

SpigotMC#

#

SpigotMC++

#

SpigotMC

#

‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ ‎ A
S
S
E
SpigotMC
B
L
Y

#

Spigot is a multi-language minecraft server 😎

#

Btw, I've made some trashy command execution for a json database engine I've made
https://github.com/KarmaDeb/KarmaAPI2/blob/master/KarmaAPI-Core/src/main/java/es/karmadev/api/database/model/json/JsonConnectionExecutor.java

Any suggestion about improving it?
(other than simplifying and splitting into methods, which I'm currently working on)

GitHub

KarmaAPI2/KarmaAPI-Core/src/main/java/es/karmadev/api/database/mode...

KarmaAPI2 is the new version of the KarmaAPI. Contribute to KarmaDeb/KarmaAPI2 development by creating an account on GitHub.

jagged rock Oct 24, 2023, 11:35 PM

#

ow what the fuck

night knoll Oct 24, 2023, 11:35 PM

#

Yes

jagged rock Oct 24, 2023, 11:35 PM

#

there's a lot of room for improvement

#

thinking of a Map<String, Predicate<String>> comparators

#

and some other stuff

#

not a fan

night knoll Oct 24, 2023, 11:38 PM

#

Wait, how could I use predicate instead of patterns for it?

#

No way, I just realised

#

You are right

#

Anyway, there're some things I still need from Pattern, such as grouping

round fossil Oct 25, 2023, 12:49 AM

#

night knoll Anyway, there're some things I still need from Pattern, such as grouping

Cache them

#

And use named groups

#

Because you have so many

night knoll Oct 25, 2023, 1:16 AM

#

Oh, completely forgot naming groups was a featur

#

Anyway, any other recomendation or bad practice that I'm doing?

round fossil Oct 25, 2023, 1:29 AM

#

There is a convention that a regex should be as strict as possible

#

So that ofc means avoid wildcard matching if possible

#

mainly because regex that matches against a lot is easier to break, since there are plausibly more edge cases around that pattern

#

(Tho this is mostly important if the regex is used to match end user input)

willow bone Oct 25, 2023, 7:56 AM

#

round fossil Since its a derivative of JavaPlugin, where the common term is Plugin (it even a...

SneakyIllegalRuntimeIOException

#

perfect

round fossil Oct 25, 2023, 11:39 AM

#

willow bone SneakyIllegalRuntimeIOException

UncheckedIOException exists

night knoll Oct 25, 2023, 11:06 PM

#

https://github.com/KarmaDeb/KarmaAPI2/tree/master/KarmaAPI-Spigot/src/main/java/es/karmadev/api/spigot/command
Simple command framework for spigot, it allows you to create sub-commands in a easy and "native" way

GitHub

KarmaAPI2/KarmaAPI-Spigot/src/main/java/es/karmadev/api/spigot/comm...

KarmaAPI2 is the new version of the KarmaAPI. Contribute to KarmaDeb/KarmaAPI2 development by creating an account on GitHub.

night knoll Oct 26, 2023, 6:44 PM

#

https://github.com/skinnynoonie/CharacterWar

I'm aware of the lack of docs in the readme

GitHub

GitHub - skinnynoonie/CharacterWar: A Minecraft Bukkit plugin.

A Minecraft Bukkit plugin. . Contribute to skinnynoonie/CharacterWar development by creating an account on GitHub.

#

but I would like some help with organizing this and would like some opinions

woeful pasture Oct 26, 2023, 6:44 PM

#

public static CharacterWar getInstance() {
return JavaPlugin.getPlugin(CharacterWar.class);
}

#

Don't ever get instance like this

#

If you're going to use q singleton do it properly

private static CharactarWars instance

onEnable()
 instance = this

night knoll Oct 26, 2023, 6:46 PM

#

that's how I used to do it

#

but idk I think I've seen a forum saying u could do that too

woeful pasture Oct 26, 2023, 6:46 PM

#

That's how it should be done if you're going to use the main clas sas a singleton

night knoll Oct 26, 2023, 6:46 PM

#

but what's bad about it

woeful pasture Oct 26, 2023, 6:47 PM

#

Whoever said that on the forums is an idiot

#

I'd have to pull up source but iirc it uses reflection

#

Forget how that's stored exactly but it'll always be better to do it the way I provided

pine grail Oct 26, 2023, 6:47 PM

#

woeful pasture I'd have to pull up source but iirc it uses reflection

doesn't

night knoll Oct 26, 2023, 6:47 PM

#

What if it's just a map of class and instance

woeful pasture Oct 26, 2023, 6:48 PM

#

Still would be better to use the way provided above

pine grail Oct 26, 2023, 6:48 PM

#

it gets the classloader of the provided class, checks if it is a plugin class loader and if it is it gets the stored plugin instance

night knoll Oct 26, 2023, 6:49 PM

#

ok

woeful pasture Oct 26, 2023, 6:52 PM

#

public boolean itemIsSimilar(ItemStack itemOne, ItemStack itemTwo) { foe this method in AbstractCharacter don't replace the item with air if it's null if you know either one Is air just return false. The method doesn't seem like it's in the right place to begin with though it should probably be in a utility class

night knoll Oct 26, 2023, 6:53 PM

#

ok

woeful pasture Oct 26, 2023, 6:53 PM

#

You also really shouldn't be comparing items with display name you should be using PDC

night knoll Oct 26, 2023, 6:53 PM

#

I'm not gonna remove the null part

woeful pasture Oct 26, 2023, 6:53 PM

#

?pdc

rich fogBOT Oct 26, 2023, 6:53 PM

#

https://www.spigotmc.org/threads/a-guide-to-1-14-persistentdataholder-api.371200/

night knoll Oct 26, 2023, 6:53 PM

#

cause bukkit api is inconsistent with AIR and null

woeful pasture Oct 26, 2023, 6:55 PM

#

That is true but you can remove the assignment to air and still ensure the end method contract is the same

#

public void registerCharacter(Character character) { try { in this method don't try catch class cast exception. You can just do an instance of check. Or better yet if every character will have a listener have interfaceCharacter extend Listener instead of having AbstractCharacter implement it

night knoll Oct 26, 2023, 6:57 PM

#

it doesn't make sense for Character to extend it tho

woeful pasture Oct 26, 2023, 6:58 PM

#

It does based on how that method works

#

You're throwing if it's not a listener

#

If it's not unexpected do instanceof and don't throw an exception

night knoll Oct 26, 2023, 6:59 PM

#

I thought about splitting the Character where Character is like the information class, and then it also had a CharacterItemSet where it implements all the items

night knoll Oct 26, 2023, 7:56 PM

#

@woeful pasture

#

what do you recommend doing on getting the pdc

#

cause each every will trigger and try to filter the pdc

#

so there's going to be like 50 characters all loading the pdc and only 1 of them does the actual functioning

woeful pasture Oct 26, 2023, 8:17 PM

#

night knoll cause each every will trigger and try to filter the pdc

Getting the PDC is not much less efficient than getting the name

#

The change in performance is negligible

night knoll Oct 26, 2023, 8:18 PM

#

no I mean like

#

lets say I have 50 characters

#

and each one of them is a registered listener that all get the pdc in the event

#

that's getting the pdc 50 times

woeful pasture Oct 26, 2023, 8:19 PM

#

It's no worse then getting the display name 50 times

#

Though slightly higher memory overhead which is negligible on modern systems

night knoll Oct 26, 2023, 8:19 PM

#

bro

#

im saying

#

im not comparing the two

#

im saying will my server lag if I do that

woeful pasture Oct 26, 2023, 8:20 PM

#

You also shouldn't need to grab the PDC for every character it should be done in O(1) operation you need to know which character a given player has equipped

#

That way you're not filtering 50 characters instead your simply doing it once for the character you know it is

night knoll Oct 26, 2023, 9:58 PM

#

woeful pasture That way you're not filtering 50 characters instead your simply doing it once fo...

no?

#

people can mix and match the character item pieces

woeful pasture Oct 26, 2023, 10:00 PM

#

the way you represent your data is then misleading and not ideal. You'd really want a some data structure of Custom Items rather than doing something like a Character interface.

You can represent this data by making some form of registry.
Map<KeyValue, ItemData>

To further expound. You really should be handling these items like Custom Items rather than a complete set if they can be mixed and matched. Each ItemData can have a handle for an event. Note you really shouldn't register these as event handlers rather call them when they're needed within a core listener for example

public onEquip(PlayerAmrmorEquipEvent event) {
  key = event.getPlayer.getHelmet().geOurSpecialKey();
  OurItemCache.getItem(key).callEvent(event);
}```

#

in this case you could likely get away with only using a String for the KeyValue and just putting the given string in PDC

#

then you have a constant key which holds the item ID

#

e.g. item-id = speedy_boots or item-id = wings

night knoll Oct 26, 2023, 10:03 PM

#

alr

#

but how should I pass the event over

#

here's what I was thiking

woeful pasture Oct 26, 2023, 10:05 PM

#

check updated message above

night knoll Oct 26, 2023, 10:05 PM

#

oh

#

yeah that was how I thought about doing it

woeful pasture Oct 26, 2023, 10:05 PM

#

just note that the above code is sudo code so it won't work

night knoll Oct 26, 2023, 10:05 PM

#

class CustomItemEventAdapter {
  public void onDamageEvent(EntityDamageEvent event) {
  }
}

#

and should I just have all the events

#

wait what

#

u can use .callEvent

#

wtf

#

never knew that

woeful pasture Oct 26, 2023, 10:05 PM

#

no no

#

lol

night knoll Oct 26, 2023, 10:05 PM

#

oh

woeful pasture Oct 26, 2023, 10:05 PM

#

Its sudo code it won't work

night knoll Oct 26, 2023, 10:05 PM

#

night knoll ```java class CustomItemEventAdapter { public void onDamageEvent(EntityDamageE...

should I do that

#

I think Java Discord API does that

woeful pasture Oct 26, 2023, 10:08 PM

#

night knoll ```java class CustomItemEventAdapter { public void onDamageEvent(EntityDamageE...

yeah pretty much sometihng along those lines. Just remember you should be doing something like this for your custom item class though

public abstract class CustomItem {
  ... // blah blah other data here that is consistent against all other items
  public void trigger(PlayerEvent event) // you can use generics here or just case {
    // DO STUFF WOOOO
  }
}```

night knoll Oct 27, 2023, 1:43 AM

#

@woeful pasture

#

The CustomItem class should also have a .getItem() right

woeful pasture Oct 27, 2023, 1:47 AM

#

it should return a copy of whatever your base item would be

#

ItemStacks are mutable so it has to be a copy

night knoll Oct 28, 2023, 2:15 AM

#

@woeful pasture

#

bruh

#

do I need to dependency inject my namespaced key

#

to all the items

woeful pasture Oct 28, 2023, 2:29 AM

#

wtf no, what are you even doing you can just create Namespaced keys on the fly

#

tbh I'd save them as static variables

night knoll Oct 28, 2023, 2:41 AM

#

k

night knoll Oct 29, 2023, 3:50 AM

#

https://github.com/skinnynoonie/CharacterWar

Did some re modeling

GitHub

GitHub - skinnynoonie/CharacterWar: A Minecraft Bukkit plugin.

A Minecraft Bukkit plugin. . Contribute to skinnynoonie/CharacterWar development by creating an account on GitHub.

#

I use PDCs now, and each item has their own class for their own impl

#

Things I will be doing so dw abt them:

Documentation
Command API
Maybe configurable messages

willow bone Oct 29, 2023, 11:48 AM

#

jagged rock and has nothing to do with skyblock

It is a remake of hypixel skyblock. Not normal skyblock

willow bone Oct 29, 2023, 11:49 AM

#

sly jackal I would just use DI if you can

What's that?

sly jackal Oct 29, 2023, 11:49 AM

#

dependency injection

#

?DI

#

I guess commands dont work here

rancid fern Oct 29, 2023, 11:52 AM

#

?di

rich fogBOT Oct 29, 2023, 11:52 AM

#

Guide to dependency injection: https://www.spigotmc.org/wiki/using-dependency-injection/

sly jackal Oct 29, 2023, 11:52 AM

#

oof its case sensitive

willow bone Oct 29, 2023, 12:11 PM

#

sly jackal dependency injection

oh I see

#

I don't really save mobs in a list

#

I save their constructors

#

and same for items

#

The mobs's NMS implementation implement a custom interface to get the SkyblockMob class

#

I don't use mappings of MOBS to UUIDS or any bs

night knoll Oct 29, 2023, 10:04 PM

#

night knoll https://github.com/skinnynoonie/CharacterWar Did some re modeling

can someone review

#

please

night knoll Oct 29, 2023, 11:14 PM

#

middle

#

depends

jagged rock Oct 29, 2023, 11:17 PM

#

Don't expose mutable collections (CharacterManager#getCharacters should expose an immutable copy)
Null validation for getCharacterByReferenceName
I'd make a dummy CustomItemImpl rather than returning null.
I'd make a ConfigurableCharacter class and load characters from the config, there's no need for them to be hardcoded
Messages could be configurable
You have a MessageConstants class yet keep your message constants in your command class
ItemListenerAdapter could be replaced with a fancy generified subscribe(eventClass, eventConsumer) method
I have no idea what's going on the CustomEntityDamageEvent, the class name suggest it's an event class but it's an interface/data class combo
Your Commands class doesn't check if the command is registered in the plugin.yml, error prone

#

Bunch more stuff, I'd structure the project in an entirely different way so I don't want to start nitpicking

round fossil Oct 29, 2023, 11:20 PM

#

the name should be instance, not INSTANCE
use this whenever possible, it adds readability
weird usage of packages, me.skinnynoonie.characterwar.character.impls and me.skinnynoonie.characterwar.character have no relationship, don't treat them as child-parent
your character manager class is just a repository class, name it more precisely
the term "reference name" is weird, find a better way to convey the meaning behind whatever that is
your cooldown manager class has a static method over the constructor, switch order
use variables when you know the instance is gonna be called multiple times

        if (cooldowns.get(uuid) == null) {
            return true;
        }
        if (cooldowns.get(uuid).get(key) == null) {
            return false;
        }
        return cooldowns.get(uuid).get(key).expired();
``` very little reason to do this.
- name packages singularly
- ur commands class is not a util class
- you use singletons and dependency injection oddly, go with dependency injection if u feel unsure what to use
- your item listener adapter isnt an adapter class
- your custim item impl class isnt an implementational class
- CMD is a weird acronym suffix for a class, consider writing it out

night knoll Oct 30, 2023, 12:29 AM

#

holy moly

#

@round fossil

#

https://ci.dv8tion.net/job/JDA/javadoc/net/dv8tion/jda/api/hooks/ListenerAdapter.html

#

JDA did this so i thought to do the same

round fossil Oct 30, 2023, 12:30 AM

#

that class is not a primary example lmao

#

just because they do it doesnt mean its inherently used correctly

#

anyway, @night knoll i dont know if they use the pattern correctly (but you were not doing it at least)

#

yeah I just read that code

#

it looks like it does something more than just being an interface with a set of abstract methods (the reflection at the very bottom of the source file)

night knoll Oct 30, 2023, 12:37 AM

#

bruh

round fossil Oct 30, 2023, 12:38 AM

#

its really a trap, even relatively well known projects at times may have poor naming choices

night knoll Oct 30, 2023, 12:38 AM

#

Yo I'll be back tmr guys

round fossil Oct 30, 2023, 12:38 AM

#

yea gl :)

night knoll Oct 30, 2023, 12:38 AM

#

@round fossil

#

What do I call the class then

#

cause I like it how the method name correlates with the impl

round fossil Oct 30, 2023, 12:38 AM

#

just name it like ItemListener, ItemObserver, ItemHandler or sth idk

#

just as long as the name is not misleading or irrelevant

night knoll Oct 30, 2023, 12:39 AM

#

But it deals with abstraction for events

round fossil Oct 30, 2023, 12:39 AM

#

yes I see that

night knoll Oct 30, 2023, 12:39 AM

#

ItemListenerMethods.class

round fossil Oct 30, 2023, 12:39 AM

#

but its not adapter just because of "But it deals with abstraction for events"

#

no no no

#

just ItemListener is fine

night knoll Oct 30, 2023, 12:40 AM

#

CustomItemListener

round fossil Oct 30, 2023, 12:41 AM

#

an example from mojang (a bit more... well... correct one) is the nms entity Villager
it implements

public interface ReputationEventHandler {
    public void onReputationEventFrom(ReputationEventType var1, Entity var2);
}

#

which is some type of even listener abstraction

night knoll Oct 30, 2023, 12:41 AM

#

Is the Mc src good

round fossil Oct 30, 2023, 12:41 AM

#

not always

night knoll Oct 30, 2023, 12:41 AM

#

like organized

#

how tho

round fossil Oct 30, 2023, 12:41 AM

#

they have bombardered their code with design patterns, basically overengineered it

night knoll Oct 30, 2023, 12:41 AM

#

isn't there professional devs and stuff

round fossil Oct 30, 2023, 12:42 AM

#

yes, but that doesn't mean you need to overengineer everything

#

iirc at one point they had a singleton for sneaky throwing

night knoll Oct 30, 2023, 12:42 AM

#

@round fossil

round fossil Oct 30, 2023, 12:42 AM

#

yea

night knoll Oct 30, 2023, 12:42 AM

#

So

round fossil Oct 30, 2023, 12:42 AM

#

like skinnynoonie, let me just get this straight with u

night knoll Oct 30, 2023, 12:43 AM

#

ImIllusion said my custom event impls are misleading

round fossil Oct 30, 2023, 12:43 AM

#

there is no src thats good to the very end

night knoll Oct 30, 2023, 12:43 AM

#

what and how should I call and organize them

#

All they do is send events to the ItemListeners, so I don't spam get the PDC

round fossil Oct 30, 2023, 12:43 AM

#

yes its misleading

#

because in bukkit you'd basically think its one of those PlayerJoinEvent, PlayerQuitEvent etc

night knoll Oct 30, 2023, 12:44 AM

#

PlayerInteractItemListenerImpl

round fossil Oct 30, 2023, 12:44 AM

#

uh sure

round fossil Oct 30, 2023, 12:45 AM

#

night knoll what and how should I call and organize them

one good starting point is to get rid of poorly named classes, variables and packages

#

second is to not take code for granted

#

just because its written by a reputable developer doesnt mean its impeccable

#

try to be critical, what would I have done here? what do I not like here? what do I like here? why is this good/bad?

night knoll Oct 30, 2023, 12:46 AM

#

how should I organize my characters

#

cause character. Is for all the managers and other stuff

#

And impls. I also thought was kinda bad

round fossil Oct 30, 2023, 12:46 AM

#

ur character class sucks

night knoll Oct 30, 2023, 12:46 AM

#

But I also thought characters. Would be too long

round fossil Oct 30, 2023, 12:46 AM

#

u use abstract classes wrongly

night knoll Oct 30, 2023, 12:47 AM

#

well it is abstract

round fossil Oct 30, 2023, 12:47 AM

#

and?

#

the abstract there doesn't achieve something meaningful in comparison to other semantic features

#

like

#

you probably just want an interface

#

and an impl

#

w/o the abstract

#

i am gonna get hate for this one @night knoll , but I generally believe abstract classes are one of the easiest tools to make ur code smell in java

#

they allow extremely tightly coupling in the shape of parent-child dependence

#

if possible, use interfaces with default methods and then just concrete implementations

night knoll Oct 30, 2023, 12:50 AM

#

round fossil you probably just want an interface

nah I did that

round fossil Oct 30, 2023, 12:50 AM

#

public abstract class Character {

    private final CustomItemImpl helmet;
    private final CustomItemImpl chestplate;
    private final CustomItemImpl leggings;
    private final CustomItemImpl boots;
    private final List<CustomItemImpl> items;

    public Character() {
        helmet = getHelmetImpl();
        chestplate = getChestplateImpl();
        leggings = getLeggingImpl();
        boots = getBootsImpl();
        items = getItemImpls();
    }

    public abstract @NotNull String getReferenceName();

    public abstract @NotNull String getDisplayName();

    public abstract @NotNull String getDescription();

    public abstract @NotNull CharacterVerse getVerse();

    public abstract @Nullable CustomItemImpl getHelmetImpl();

    public abstract @Nullable CustomItemImpl getChestplateImpl();

    public abstract @Nullable CustomItemImpl getLeggingImpl();

    public abstract @Nullable CustomItemImpl getBootsImpl();

    public abstract @NotNull List<@NotNull CustomItemImpl> getItemImpls();

#

this is not an interface

night knoll Oct 30, 2023, 12:51 AM

#

and the reason I didn't want to use an interface because I didn't think it was worthy to make many instances of an item impl

#

look at the pre-made methods

round fossil Oct 30, 2023, 12:51 AM

#

but there is more commitment to an abstract class than to an interface tho?

#

you're coping rn

night knoll Oct 30, 2023, 12:51 AM

#

what

#

I used an interface first

#

Is what I ment

round fossil Oct 30, 2023, 12:51 AM

#

that was a good choice

#

idk why u switched

night knoll Oct 30, 2023, 12:52 AM

#

I don't think it makes sense to basically do new SupermanLeggingsImpl().getItem(); every time

#

someone gets a kit

round fossil Oct 30, 2023, 12:52 AM

#

I mean you would cache SupermanLeggingsImpl surely?

night knoll Oct 30, 2023, 12:52 AM

#

how

round fossil Oct 30, 2023, 12:53 AM

#

throw away objects is generally considered an anti pattern

#

i mean just create a repository class

night knoll Oct 30, 2023, 12:53 AM

#

What's that

round fossil Oct 30, 2023, 12:53 AM

#

a class whose sole purpose is to allow access to data(-transfers) somehow (often through key-value)

#

for instance

#

class UserRepository {
  Map<String,User> backingMap; //with getters and setters without exposing map
}
class CharacterTypeRepository {
  Map<String,CharacterType> backingMap; // -||-
}
class Character {
  CharacterType type;
  User owner;
}
class User {
  Map<String,Character> userCharacters;
  Character currentCharacter;
}

night knoll Oct 30, 2023, 12:56 AM

#

or I can use an abstract class

#

Ppl can mix and match character pieces btw

round fossil Oct 30, 2023, 12:58 AM

#

then sth like

UserRepository userRepo = ...;
CharacterTypeRepository characterTypeRepo = ...;
Storage storage = ...;
User user = userRepo.findUser(uuid);
storage.loadData(user).await();
Character character = user.createNewCharacter("name", characterTypeRepo.getByName("superman"));
user.selectCharacter(character);
storage.saveData(user).await();

night knoll Oct 30, 2023, 12:59 AM

#

sorry but I ain't doing all that

round fossil Oct 30, 2023, 12:59 AM

#

interface CharacterType {
  //an optional accessor
  @Nullable public default String description() {
    return null;
  }

  //a required accessor
  public String name();
}```

#

u dont have to do all that

#

but u get the idea

night knoll Oct 30, 2023, 1:00 AM

#

yes I do

#

Motivated me to use my abstract class more

round fossil Oct 30, 2023, 1:00 AM

#

you asked for a code interview, I will point out code smells based on where ur code is most likely gonna be an issue in the future or for ppl who read in the future

#

if u wna apply it or not is up to u

night knoll Oct 30, 2023, 1:01 AM

#

ik how

round fossil Oct 30, 2023, 1:01 AM

#

as said the issue with abstract classes is that they cause TIGHT COUPLINESS

night knoll Oct 30, 2023, 1:01 AM

#

Character interface
AbstractCharacter abstract class
BatmanCharacter impl

round fossil Oct 30, 2023, 1:01 AM

#

and when they dont cause tight coupliness they basically just do what interfaces do already

night knoll Oct 30, 2023, 1:01 AM

#

boom

round fossil Oct 30, 2023, 1:01 AM

#

lol why the AbstractCharacter

night knoll Oct 30, 2023, 1:01 AM

#

Cause u like ur interface so much

round fossil Oct 30, 2023, 1:01 AM

#

ur character is just a fkn data transfer anyway

#

?

#

no im asking u, its not about me

night knoll Oct 30, 2023, 1:02 AM

#

It's a data holder

#

and it's just a simple chache

#

cache

round fossil Oct 30, 2023, 1:02 AM

#

yes so why would u have an abstract class for it

#

there is nothing to make abstract in the first place

night knoll Oct 30, 2023, 1:02 AM

#

not if I make the interface

round fossil Oct 30, 2023, 1:02 AM

#

u dont get the point

#

ill re-iterate myself

#

abstract classes are one of the easiest tools to make ur code smell in java

they allow extremely tightly coupling in the shape of parent-child dependence

#

and when they dont cause tight coupliness they basically just do what interfaces do already

night knoll Oct 30, 2023, 1:03 AM

#

What does tight coupling mean

round fossil Oct 30, 2023, 1:03 AM

#

it means code is tightly dependent on the components it depends on

night knoll Oct 30, 2023, 1:03 AM

#

What does it depend on

round fossil Oct 30, 2023, 1:04 AM

#

for instance if ur code is binded to an instance (like a singleton) then its tightly coupled

night knoll Oct 30, 2023, 1:04 AM

#

it's not dependent on the abstract class

round fossil Oct 30, 2023, 1:04 AM

#

Bukkit.getServer().getOnlinePlayers() is a tightly coupled method call, because whatever class that calls this method depends on an instance

round fossil Oct 30, 2023, 1:05 AM

#

night knoll it's not dependent on the abstract class

no but the issue with abstract classes is that all subclasses of that abstract class are gonna be extremely dependent on the concretion of the abstract class to work accordingly

night knoll Oct 30, 2023, 1:05 AM

#

No

round fossil Oct 30, 2023, 1:05 AM

#

which means the maintenance level just increase signficantly

#

yes lol

night knoll Oct 30, 2023, 1:05 AM

#

How

round fossil Oct 30, 2023, 1:05 AM

#

quite literally

#

no but the issue with abstract classes is that all subclasses of that abstract class are gonna be extremely dependent on the concretion of the abstract class to work accordingly

night knoll Oct 30, 2023, 1:05 AM

#

Send me the superman character class

round fossil Oct 30, 2023, 1:06 AM

#

urs?

night knoll Oct 30, 2023, 1:06 AM

#

Yes I'm on phone

round fossil Oct 30, 2023, 1:06 AM

#

https://github.com/skinnynoonie/CharacterWar/blob/main/src/main/java/me/skinnynoonie/characterwar/character/impls/SupermanCharacter.java

night knoll Oct 30, 2023, 1:06 AM

#

Like in dc

round fossil Oct 30, 2023, 1:06 AM

#

no thats spammy

night knoll Oct 30, 2023, 1:06 AM

#

Alr hold on

round fossil Oct 30, 2023, 1:06 AM

#

dont send it

#

just read it instead

night knoll Oct 30, 2023, 1:07 AM

#

I'm brushing my teeth

round fossil Oct 30, 2023, 1:08 AM

#

as said again, THE REASON YOU DONT EXPERIENCE THE ISSUES WITH ABSTRACT CLASSES IS:

your code is like 500 lines in total, u dont have any issues changing stuff because the project is so small
you dont even use the abstract class in a way that it utilizes the way abstract classes are supposed to be used, your implementation sucks, it can be implemented with just interfaces and normal classes

night knoll Oct 30, 2023, 1:08 AM

#

What issue am I gunna

#

Run into

#

Doing that

round fossil Oct 30, 2023, 1:08 AM

#

also because you don't really understand the difference between type dependence and unit dependence

#

prob no issues unless u wanna scale ur project

#

but u went here for a code review

#

so here u get one

#

the way u used the abstract class smells, no matter how u try to advert ur eyes, im not trying to insult you, but im just emphasizing there are better ways to do what you did there

#

if you dont care about improving the quality of code there is really no point in asking for a review

night knoll Oct 30, 2023, 1:10 AM

#

How tho

round fossil Oct 30, 2023, 1:10 AM

#

with an interface

night knoll Oct 30, 2023, 1:10 AM

#

bro

round fossil Oct 30, 2023, 1:10 AM

#

quite literally

#

dont bro me, you know what I mean and what I have been implying all this time

night knoll Oct 30, 2023, 1:11 AM

#

Buddy

#

the way ur doing it

#

is over engineering it

round fossil Oct 30, 2023, 1:11 AM

#

its really not

night knoll Oct 30, 2023, 1:11 AM

#

I don't rly see how ima run into any issues

#

the Character abstract class is just a data holder class

#

There's no impl

#

nothing

#

it's like an artifact at a museum

round fossil Oct 30, 2023, 1:12 AM

#

you dont even know what impl means

night knoll Oct 30, 2023, 1:12 AM

#

don't start insulting me

round fossil Oct 30, 2023, 1:12 AM

#

im not

night knoll Oct 30, 2023, 1:12 AM

#

the only tight coupling that's gonna happen is between me and u

round fossil Oct 30, 2023, 1:13 AM

#

you drop this, dont ask for code reviews again if you're going to behave like this

night knoll Oct 30, 2023, 1:13 AM

#

Explain to me

#

Why doing it your way is better

#

like I still don't get why I can't just use an abstract class for this

round fossil Oct 30, 2023, 1:14 AM

#

its not like you cant

#

i say it again, there are better ways to do it

#

one is with just an interface

night knoll Oct 30, 2023, 1:15 AM

#

round fossil ```java class UserRepository { Map<String,User> backingMap; //with getters and...

idk why u have user class and why a character class has a owner

#

And then a String, User map

round fossil Oct 30, 2023, 1:15 AM

#

idk I just made it up

#

my example is irrelevant

night knoll Oct 30, 2023, 1:15 AM

#

... ok

#

ok so let's say I use an interface

#

What am i gonna do about those throw away objects

round fossil Oct 30, 2023, 1:17 AM

#

Abstract classes promote what's called implementation inheritance which is when sub classes strictly rely on their parent class structures, the means when the parent class implementation is changed, the subclass has to change accordingly. If this does not apply to your abstract class then you use your abstract class wrongly, which means it can simply be done with an interface and some concrete classes. If this does apply to your abstract class then you have a proper use of the abstract class, but it comes with the cost of this tight sub-super class relationship.

round fossil Oct 30, 2023, 1:17 AM

#

night knoll What am i gonna do about those throw away objects

whether u want to have throw away objects or not is independent of if u use an abstract class or an interface

night knoll Oct 30, 2023, 1:18 AM

#

round fossil Abstract classes promote what's called implementation inheritance which is when ...

ok so

#

Character - interface
AbstractCharacter - abstract class
SupermanCharacter - impl

round fossil Oct 30, 2023, 1:18 AM

#

no

#

remove the abstract charcter

#

its not needed

night knoll Oct 30, 2023, 1:18 AM

#

Character changes, abstractcharcter will too

round fossil Oct 30, 2023, 1:19 AM

#

yes but if Character changes, the the contract of ur component's functionality is changed

#

so of course the underlying implementation has to adapt to the change

#

im talking about implementation changes

#

the fragile parts of code

#

AbstractMap, AbstractList etc exists because they are skeletal implementations
(no one ever depends on these types, they are just there for convenience without damaging maintainability, structure etc altho it does affect flexibility)

#

yours is not

night knoll Oct 30, 2023, 1:20 AM

#

round fossil yes but if Character changes, the the contract of ur component's functionality i...

No?

round fossil Oct 30, 2023, 1:20 AM

#

yes it does

night knoll Oct 30, 2023, 1:20 AM

#

how

#

Let's say I get a

round fossil Oct 30, 2023, 1:21 AM

#

because its an interface

#

an interface is the functionality of something

night knoll Oct 30, 2023, 1:21 AM

#

ok so I use an abstract class

#

cause it has no functionality

#

Other than displaying sata

#

data

round fossil Oct 30, 2023, 1:21 AM

#

why?

#

you dont need the fucking abstract class

#

it does nothing

night knoll Oct 30, 2023, 1:21 AM

#

bro chill out

round fossil Oct 30, 2023, 1:22 AM

#

this is not getting anywhere

night knoll Oct 30, 2023, 1:22 AM

#

yeah because what am I gonna do

#

Without the abstract class

round fossil Oct 30, 2023, 1:22 AM

#

just dont use the abstract class?

#

i m not gonna explain anything further

night knoll Oct 30, 2023, 1:22 AM

#

Then what do I do about the throw away objects

round fossil Oct 30, 2023, 1:22 AM

#

you refuse to listen anyway

night knoll Oct 30, 2023, 1:22 AM

#

u never answered that question

round fossil Oct 30, 2023, 1:22 AM

#

dont throw them away

#

cache them?

night knoll Oct 30, 2023, 1:23 AM

#

bruh

#

how

round fossil Oct 30, 2023, 1:23 AM

#

by mapping them?

#

this is basic data design

night knoll Oct 30, 2023, 1:23 AM

#

how would that look like

round fossil Oct 30, 2023, 1:23 AM

#

look at my example for instance lmao

#

u have a map with keys, and then normal plain java classes

night knoll Oct 30, 2023, 1:24 AM

#

CharacterRegistry - sort manager class?
Character - interface
SupermanCharacter - impl

round fossil Oct 30, 2023, 1:24 AM

#

yes

night knoll Oct 30, 2023, 1:24 AM

#

like

round fossil Oct 30, 2023, 1:24 AM

#

thats good

night knoll Oct 30, 2023, 1:24 AM

#

bruh

#

And then

#

CharacterRegistry{

getLeggings(Class<? extends Character>)

}

#

?

round fossil Oct 30, 2023, 1:25 AM

#

you can make it surjective sort of I suppose

#

like

night knoll Oct 30, 2023, 1:28 AM

#

Like what

round fossil Oct 30, 2023, 1:28 AM

#

CharacterRegistry {
IdentityHashMap<Type,Character> mapByType;
HashMap<String,Character> mapByName;
HashMap<Integer,Character> mapById;
int id = 0;

void register(Character character) {
mapByType.put(character.getClass(),character);
mapByName.put(character.name(),character);
mapById.put(id++,character);
}

//get by id
Character byId(int id) {
return mapById.get(id);
}

//get by name
Character byName(String name) {
return mapByName.get(name);
}

//get by type
<C extends Character> byType(Class<C> type) {
return (C) mapByType.get(type);
}
}

#

example only

night knoll Oct 30, 2023, 1:29 AM

#

ok bro what

round fossil Oct 30, 2023, 1:29 AM

#

im just trying to give u an idea or well inspiration

night knoll Oct 30, 2023, 1:29 AM

#

I'm tryna get the leggings

#

without making a throw away object

round fossil Oct 30, 2023, 1:29 AM

#

yea

#

so when u need the leggings u'd just:
registry.byName("superman").getLeggings();

night knoll Oct 30, 2023, 1:30 AM

#

Not possible

round fossil Oct 30, 2023, 1:30 AM

#

why?

night knoll Oct 30, 2023, 1:30 AM

#

cause it won't be .getLeggings

#

The leggings are in the itemimpl class

round fossil Oct 30, 2023, 1:31 AM

#

https://github.com/skinnynoonie/CharacterWar/blob/b06065e2829744b1970296f5143886af31c85d47/src/main/java/me/skinnynoonie/characterwar/character/Character.java#L54C1-L57C1

#

?

#

im just basing myself of ur code

#

u need to figure out the design on ur own

night knoll Oct 30, 2023, 1:31 AM

#

@round fossil

#

Look at the variable

round fossil Oct 30, 2023, 1:31 AM

#

mhm

night knoll Oct 30, 2023, 1:31 AM

#

type

#

Ig that's my fault bad naming

#

but that's an impl class

round fossil Oct 30, 2023, 1:32 AM

#

the ItemStack?

#

or ru referring to CustomItemImpl?

night knoll Oct 30, 2023, 1:32 AM

#

Yes

#

The ItemStack is stored in the impl

round fossil Oct 30, 2023, 1:32 AM

#

then it would be just
registry.byName("superman").getLeggingsImpl();

#

no?

#

i mean u get the point i hope

#

reuse the objects

night knoll Oct 30, 2023, 1:33 AM

#

the getLeggingsImpl() is a throw away object

#

if u just call .getItem() right after

round fossil Oct 30, 2023, 1:33 AM

#

getItem is not a throw away object

#

because it creates an itemstack which u would most likely do something with ofc

#

itemstacks are also fine to create - use once - and repeat

night knoll Oct 30, 2023, 1:34 AM

#

night knoll the getLeggingsImpl() is a throw away object

@round fossil

round fossil Oct 30, 2023, 1:34 AM

#

its not

#

u cache it in Character.java

night knoll Oct 30, 2023, 1:35 AM

#

bro...

#

That's the purpose of my abstract class

round fossil Oct 30, 2023, 1:35 AM

#

yes but u dont need abstract for that lol

night knoll Oct 30, 2023, 1:35 AM

#

yo I'm ngl

#

we are in circles again

round fossil Oct 30, 2023, 1:36 AM

#

yes because you base yourself around the importance of that the abstract class is useful because its abstract or something

night knoll Oct 30, 2023, 1:36 AM

#

It is abstract tho

round fossil Oct 30, 2023, 1:36 AM

#

ru trolling?

night knoll Oct 30, 2023, 1:36 AM

#

tell me how it's not abstract

#

no I'm being serious

round fossil Oct 30, 2023, 1:36 AM

#

yes because you base yourself around the importance of that the abstract class is useful because its abstract or something

night knoll Oct 30, 2023, 1:37 AM

#

Oh

round fossil Oct 30, 2023, 1:37 AM

#

read my message pls

#

feels like u skip half of my messages

night knoll Oct 30, 2023, 1:37 AM

#

bro

#

ok so

#

let's say I use the design

#

U gave me

#

How is .getItemImpl not a throw away obj

round fossil Oct 30, 2023, 1:37 AM

#

i mean ofc u have to tweak it

night knoll Oct 30, 2023, 1:37 AM

#

I cant

round fossil Oct 30, 2023, 1:37 AM

#

let me give an example

night knoll Oct 30, 2023, 1:38 AM

#

Unless I dependency inject the itemstack into the item impl

round fossil Oct 30, 2023, 1:41 AM

#

interface Character {
  @NotNull String displayName();

  @Nullable public default CustomItemImpl leggings() { return null; }
}

class SupermanCharacter implements Character {
  private final String displayName;
  private final CustomItemImpl leggings;

  public SupermanCharacter(String displayName) {
    this.displayName = displayName;
    this.leggings = new SupermanLeggings();
  }
  
  @Override @NotNull public String displayName() { return this.displayName; }

  @Override @NotNull public CustomItemImpl leggings() { return this.leggings; }
}

#

(last line):
@Nullable @NotNull public CustomItemImpl leggings() { return this.leggings; }

if the impl does not have leggings, u just dont override the method

night knoll Oct 30, 2023, 1:42 AM

#

ok that's just stupid

round fossil Oct 30, 2023, 1:42 AM

#

what is?

night knoll Oct 30, 2023, 1:43 AM

#

instead of letting another class be the cache, the display class has to cache itself

round fossil Oct 30, 2023, 1:43 AM

#

u cache it in another class if u want lol

night knoll Oct 30, 2023, 1:43 AM

#

So now I gotta force myself to write a bunch of boiler plate

round fossil Oct 30, 2023, 1:43 AM

#

no one is restricting u

round fossil Oct 30, 2023, 1:43 AM

#

night knoll So now I gotta force myself to write a bunch of boiler plate

not rly

#

u can prob recordify it

night knoll Oct 30, 2023, 1:43 AM

#

How is it not

#

8+ lines

#

If it has all the armor

round fossil Oct 30, 2023, 1:44 AM

#

if u dont like the boilerplate, either go with a record, or use lombok or sth like kotlin

#

abstract is not the way

#

you can prob ask any1 here, they'd agree with me (i hope at least)

night knoll Oct 30, 2023, 1:45 AM

#

@night knoll do u agree with @round fossil

#

nah I dont

round fossil Oct 30, 2023, 1:45 AM

#

not u

#

stop trolling one last time

night knoll Oct 30, 2023, 1:46 AM

#

ok here

#

How can I make the abstract class make sense

round fossil Oct 30, 2023, 1:46 AM

#

so u want it to be abstract?

night knoll Oct 30, 2023, 1:46 AM

#

yes

round fossil Oct 30, 2023, 1:47 AM

#

thats another goal, but well, idk, it doesnt really suit to be abstract there let me see

#

if u couple the event handling then maybe it would make sense

#

like let stuff such as
onPlayerDamagedWithArmor(@NotNull EntityDamageEvent event, @NotNull Player victim) be abstract methods in ur abstract class

#

and impl it in SupermanCharacter

#

i still dont like it

night knoll Oct 30, 2023, 1:48 AM

#

Nooe

#

no

#

I did that

round fossil Oct 30, 2023, 1:48 AM

#

but having that there would justify it more

night knoll Oct 30, 2023, 1:48 AM

#

I made the character impl also impl all the items

#

it was a mess

round fossil Oct 30, 2023, 1:49 AM

#

yeah so instead u resorted to an interface CustomItemImpl and let each item impl it

#

which is good design

#

i like it

night knoll Oct 30, 2023, 1:49 AM

#

Thx

round fossil Oct 30, 2023, 1:49 AM

#

the name Impl in the end is questionable since its an abstraction but w/e

#

u got the concept right

#

thats whats important

night knoll Oct 30, 2023, 1:49 AM

#

bruh rly changed pfps mid convo

round fossil Oct 30, 2023, 1:49 AM

#

me?

night knoll Oct 30, 2023, 1:49 AM

#

yes

round fossil Oct 30, 2023, 1:49 AM

#

prob my gf

#

she does weird things on my acc

#

anyway

night knoll Oct 30, 2023, 1:50 AM

#

ok well

round fossil Oct 30, 2023, 1:50 AM

#

as said

#

if u want it be abstract let it be

night knoll Oct 30, 2023, 1:50 AM

#

I didn't want a compliment

round fossil Oct 30, 2023, 1:50 AM

#

the world wont end

#

but like as mentioned, it doesnt need to be abstract and shouldnt be

night knoll Oct 30, 2023, 1:50 AM

#

ok then what should it be

round fossil Oct 30, 2023, 1:50 AM

#

normal java class

night knoll Oct 30, 2023, 1:50 AM

#

I remember someone told me

round fossil Oct 30, 2023, 1:51 AM

#

and interface

#

who?

night knoll Oct 30, 2023, 1:51 AM

#

dependency platform development

#

Where it's interfaces for everything

#

And then abstract for boiler plate

#

Then the impls

round fossil Oct 30, 2023, 1:51 AM

#

who said that

#

i will quite literally bonk them in their feet

#

its partially right

#

but its not the whole truth

#

anyway i need to sleep now

#

gn, can continue tmrw

night knoll Oct 30, 2023, 1:53 AM

#

round fossil who said that

@jagged rock I think

round fossil Oct 30, 2023, 1:54 AM

#

yes but illusion doesnt write a lot of unit tests, and havent really been in that industry, so he uses abstract classes a lot since he hasn't rly experienced the pain they may bring

jagged rock Oct 30, 2023, 1:54 AM

#

(this structure mostly works for platform-agnostic code)

night knoll Oct 30, 2023, 1:54 AM

#

ok so

round fossil Oct 30, 2023, 1:54 AM

#

if u code in mc u prob wont experience issues with abstract classes

night knoll Oct 30, 2023, 1:54 AM

#

ok so it's fine

round fossil Oct 30, 2023, 1:54 AM

#

what I gave is an industry standard review

jagged rock Oct 30, 2023, 1:54 AM

#

(where the difference between each module is about ~10 lines of code)

round fossil Oct 30, 2023, 1:55 AM

#

i mean its fine to have everything static if u're mentally unstable

night knoll Oct 30, 2023, 1:55 AM

#

bro

jagged rock Oct 30, 2023, 1:55 AM

#

(I rarely use abstract classes when working on spigot-only projects)

night knoll Oct 30, 2023, 1:55 AM

#

abstract class is abstract and has some default methods

#

it's basically an interface

round fossil Oct 30, 2023, 1:55 AM

#

not not really

#

there is a huge difference

#

fields being one of them

#

second being that interfaces are way easier to mock

#

so when it comes to transparent testing abstract classes become a huge problem

jagged rock Oct 30, 2023, 1:56 AM

#

Well in my case I make an interface and expose that interface

#

The abstract impl just de-duplicates code :)

#

If you want to mock stuff, you mock the interface

round fossil Oct 30, 2023, 1:57 AM

#

yeah its just a skeletal implementation ^

#

like AbstractMap, or w/e other abstract thingy u wanna bring up as an example

night knoll Oct 30, 2023, 1:57 AM

#

ok so lmk why this won't work (ik I've mentioned this so many times)

Character - interface
AbstractCharacter - abstract class
SupermanCharacter - impl

Has an interface so it encapsulates a lot of stuff

Abstract is abstract, has some boiler plate reduction

Impl - the impl

round fossil Oct 30, 2023, 1:57 AM

#

yes but u have just data

#

so the abstract there is pointless

night knoll Oct 30, 2023, 1:58 AM

#

since when was abstraction not allowed for data

round fossil Oct 30, 2023, 1:58 AM

#

didnt say disallowed

#

i said pointless

jagged rock Oct 30, 2023, 1:58 AM

#

There are use-cases

night knoll Oct 30, 2023, 1:58 AM

#

@jagged rock how would u do this

jagged rock Oct 30, 2023, 1:58 AM

#

I'd honestly make the entire Character part just interfaces

#

And config files that point to abstract items

#

but tbh you barely need an abstract class for custom item logic

round fossil Oct 30, 2023, 1:59 AM

#

^

night knoll Oct 30, 2023, 1:59 AM

#

its not even the logic

#

it's just for boiler plate

round fossil Oct 30, 2023, 2:00 AM

#

yes but boilerplate is invalid reason

jagged rock Oct 30, 2023, 2:00 AM

#

well

#

yes and no

round fossil Oct 30, 2023, 2:00 AM

#

if u wanna avoid repeating yourself

night knoll Oct 30, 2023, 2:00 AM

#

I'll try to add the custom configurable items thing

#

See what happens

jagged rock Oct 30, 2023, 2:00 AM

#

I use abstraction for DRY reasons

round fossil Oct 30, 2023, 2:00 AM

#

then yes use abstract to skeletally abstract away repetitions

#

yes but boilerplate is something else

#

java has it by nature

#

if u wanna avoid it, use lombok or kotlin ig

night knoll Oct 30, 2023, 2:00 AM

#

switching to C

#

bye guys

jagged rock Oct 30, 2023, 2:01 AM

#

(ignore the broken imports)
This is how I do cosmetics

#

but like

#

if I were to make it now this would be an interface

round fossil Oct 30, 2023, 2:02 AM

#

yeah thats more reasonable

jagged rock Oct 30, 2023, 2:02 AM

#

I wonder if I can make this an interface

#

prob not

#

this is old by my standards

night knoll Oct 30, 2023, 2:03 AM

#

yeah but u have the

round fossil Oct 30, 2023, 2:03 AM

#

anyway
avoiding boilerplate means avoiding writing out stuff like the {}, semicolons, basically unnecessary code thats just in the way and can be reduced but still produce the same semantics

avodiing repetition means you avoid writing out the same variable, the same method again etc

#

abstract class if used as a skeletal class, then it should be due to the latter

#

if u do it due to the former, then there are better tools to think about first like records, lombok, other languages or something else

jagged rock Oct 30, 2023, 2:05 AM

#

static

round fossil Oct 30, 2023, 2:06 AM

#

there is also the argument of that you should only ever abstract away if its a useful abstraction, is an abstract class for a data transfer object useful? probably not, at least 99% of the times

jagged rock Oct 30, 2023, 2:06 AM

#

For example this last class could've been reworked to proxy a Renderer interface and avoid the abstraction

night knoll Oct 30, 2023, 2:06 AM

#

bro I'm so confused

round fossil Oct 30, 2023, 2:07 AM

#

jagged rock For example this last class could've been reworked to proxy a Renderer interface...

yeah, sounds way more clean

night knoll Oct 30, 2023, 2:07 AM

#

ok so

#

nah

#

what do I even do atp

round fossil Oct 30, 2023, 2:08 AM

#

what u feel is best

jagged rock Oct 30, 2023, 2:08 AM

#

Bros about to cry in a corner out of confusion

round fossil Oct 30, 2023, 2:08 AM

#

when u understand object orientation, data orientation and design good enough you'll get it intuitively

jagged rock Oct 30, 2023, 2:08 AM

#

That's what's best

round fossil Oct 30, 2023, 2:08 AM

#

:,)

night knoll Oct 30, 2023, 2:10 AM

#

bruh

#

no

#

@round fossil @jagged rock r u guys telling me I should create the item stacks in the character impl class

jagged rock Oct 30, 2023, 2:11 AM

#

No

#

We're saying to ditch the abstract class unless there's a real good reason to keep it

round fossil Oct 30, 2023, 2:11 AM

#

generally speaking, the more responsibility given to the character class, the harder it becomes to maintain it

night knoll Oct 30, 2023, 2:14 AM

#

round fossil generally speaking, the more responsibility given to the character class, the ha...

So all the responsibility goes to the impl?

round fossil Oct 30, 2023, 2:14 AM

#

if u want

#

it makes the impl more complex and harder to maintain

#

there is pros and cons to everything

jagged rock Oct 30, 2023, 2:15 AM

#

Ever heard of Single Responsibility Principle?

#

It's a bit easy to get it wrong but basically you should write your class to serve a niche purpose

#

And nothing else

#

The more complex your project is, the more niche each class is

night knoll Oct 30, 2023, 2:16 AM

#

yeah that's what I did tho

#

the item impl is in another class

jagged rock Oct 30, 2023, 2:16 AM

#

For example, people can say "The admin command class handles everything regarding the admin command"

#

BUT

#

The admin command should only handle the command logic itself

night knoll Oct 30, 2023, 2:17 AM

#

and the abstract character handles the cache

jagged rock Oct 30, 2023, 2:17 AM

#

Any data that's manipulated by the command needs to be handled elsewhere

night knoll Oct 30, 2023, 2:17 AM

#

So I'm following srp

jagged rock Oct 30, 2023, 2:17 AM

#

depends

#

Your character class shouldn't be responsible for items

round fossil Oct 30, 2023, 2:19 AM

#

in short, srp says a class should only change its implementation for one major reason, the Database class shouldnt have to be changed because your LoginHandler needed to change for instance

night knoll Oct 30, 2023, 2:19 AM

#

jagged rock Your character class shouldn't be responsible for items

It's not

round fossil Oct 30, 2023, 2:20 AM

#

its up to the developer to define these so called major reasons

night knoll Oct 30, 2023, 2:20 AM

#

wait

#

Wdym character

#

the impl or abstract

round fossil Oct 30, 2023, 2:20 AM

#

the abstract one

night knoll Oct 30, 2023, 2:20 AM

#

Then what should

round fossil Oct 30, 2023, 2:21 AM

#

its fine if ur character class refers to WHAT items are relevant, but it shouldnt contain HOW those work or their implementations

jagged rock Oct 30, 2023, 2:22 AM

#

I'm thinking

#

public interface CharacterInventory {

  ItemStack getItem(EquipmentSlot slot);
  Collection<ItemStack> getExtraItems();

}

public interface CharacterInfo {

  String getName();
  String getDescription();

  CharacterVerse getVerse();
}

public interface Character {

  CharacterInventory getInventory();
  CharacterInfo getInfo();

}

round fossil Oct 30, 2023, 2:25 AM

#

thats really good data structuring as well

#

skinny, this is prob the type of structure u're looking for

night knoll Oct 30, 2023, 2:27 AM

#

That looks a lot more promising

#

where do I put the impl

#

Eh actually idk

#

that seems like a lot of classes for one character

jagged rock Oct 30, 2023, 2:28 AM

#

Now, you could do something like

public class BasicCharacterInfo implements CharacterInfo {

  private final String name;
  private final String description;
  private final CharacterVerse verse;

  public BasicCharacterInfo(String name, String description, CharacterVerse verse) {
    this.name = name;
    this.description = description;
    this.verse = verse;
  }

  public BasicCharacterInfo(ConfigurationSection section) {
    this.name = section.getString("name", "FIX YOUR CONFIGURATION - INVALID NAME");
    this.description = section.getString("description", "FIX YOUR CONFIGURATION - INVALID DESCRIPTION");
    this.verse = CharacterVerse.valueOf(section.getString("verse", "DC").toUpperCase(Locale.ROOT));
  }

  @Override
  public String getName() {
    return this.name;
  }

  @Override
  public String getDescription() {
    return this.description;
  }

  @Override
  public CharacterVerse getVerse() {
    return this.verse;
  }
}

night knoll Oct 30, 2023, 2:35 AM

#

since I'm using pdcs

#

how should I split the key for the actual item

jagged rock Oct 30, 2023, 2:35 AM

#

public class BasicCharacterInventory implements CharacterInventory {

  private final Map<EquipmentSlot, ItemStack> items = new HashMap<>(); // Use whatever impl you want
  private final Collection<ItemStack> extra = new ArrayList<>(36);

  public void registerEquipment(EquipmentSlot slot, ItemStack item) {
    if(slot == null || item == null) {
      return;
    }

    items.put(slot, item);
  }

  public void addExtraItems(ItemStack first, ItemStack... other) {
    if(first != null) {
      extra.add(first);
    }

    extra.addAll(List.of(other));
  }

  @Override
  public ItemStack getItem(EquipmentSlot slot) {
    ItemStack result = this.items.get(slot);
    return result == null ? null : result.clone(); // ternary, replace with if-else if you want
  }

  @Override
  public Collection<ItemStack> getExtraItems() {
    return List.copyOf(this.extra);
  }
}```

#

bim bim bam bam

#

But tbh I'd rather skip this entire inventory BS

#

Have a registry somewhere

jagged rock Oct 30, 2023, 2:37 AM

#

night knoll how should I split the key for the actual item

Your pdc string links to your custom item impl, not to the character

#

I'd probably rename CharacterInventory to just ArmorSet

#

and load and register them separate

#

linking them to a custom item registry

night knoll Oct 30, 2023, 2:39 AM

#

I mean

#

Ok how is a

#

an item impl gonna look like

jagged rock Oct 30, 2023, 2:41 AM

#

Up to you

#

You could use an abstract class for this, depending on how you plan for it to work

night knoll Oct 30, 2023, 2:41 AM

#

yay abstract class

jagged rock Oct 30, 2023, 2:44 AM

#

My usual item impls look like

public class WhateverItem extends CustomItem {

  public WhateverItem(MyPlugin plugin) {
    super(plugin);

    subscribe(PlayerInteractEvent.class, this::handleClick);
  }

  private void handleClick(PlayerInteractEvent event) {
    if(!event.getAction().isRightClick()) {
      return;
    }

    ItemStack item = event.getItem();

    if(!matches(item)) {
      return;
    }

    ...
  }

  @Getter
  public String getName() {
    return "whatever";
  }
}

#

but it can be replaced with an interface if you adventure enough

night knoll Oct 30, 2023, 2:45 AM

#

It's just matches part

#

Thats all worried about

jagged rock Oct 30, 2023, 2:45 AM

#

public interface CustomItem extends Listener {

  default boolean matches(ItemStack item) {
    String name = getName();
    // get pdc string
    return ...
  }

  String getName();
}

#

replace the subscribe with an @EventHandler

#

And register the listener when you register the item in your registry

night knoll Oct 30, 2023, 2:46 AM

#

ok but the PDC string won't be in the item

jagged rock Oct 30, 2023, 2:46 AM

#

it will

night knoll Oct 30, 2023, 2:46 AM

#

where

jagged rock Oct 30, 2023, 2:46 AM

#

in your ItemStack

#

you get the pdc

#

if it doesn't exist, it doesn't match your item

night knoll Oct 30, 2023, 2:46 AM

#

no like

#

Where do I set the pdc

jagged rock Oct 30, 2023, 2:47 AM

#

somewhere

#

here's what I do with uhh

#

abstract classes

#

https://sourceb.in/pU3DJk1QbX

#

old impl

#

wrote it a few months ago

night knoll Oct 30, 2023, 2:48 AM

#

o

#

idk

#

Ima just restart tmr

night knoll Oct 30, 2023, 9:34 PM

#

@jagged rock what do you use to config items

jagged rock Oct 30, 2023, 9:39 PM

#

polymorphism

#

I got no clue how to answer your question

night knoll Oct 30, 2023, 9:40 PM

#

jagged rock I got no clue how to answer your question

oh I meant like

#

do u use an API

jagged rock Oct 30, 2023, 9:41 PM

#

I make all my utilities

night knoll Oct 30, 2023, 9:47 PM

#

wtf

#

#

@jagged rock do u just paste your past utils into a utils package

jagged rock Oct 30, 2023, 9:48 PM

#

something like that

night knoll Oct 30, 2023, 9:48 PM

#

lmao

night knoll Oct 31, 2023, 7:32 PM

#

@jagged rock yo idk how this gonna work ima be honest

#

My abstract class way makes the most sense to me still

#

maybe I'll rename it to CharacterTemplate or something

night knoll Oct 31, 2023, 8:22 PM

#

nah nvm

woeful pasture Oct 31, 2023, 9:12 PM

#

Interfaces are pretty abstract and can be hard to think through especially if your new

#

I'd reccomend sticking with it though maybe do a formal write up for your plan

night knoll Nov 1, 2023, 1:50 AM

#

@woeful pasture

#

I talked to someone and they said abstract classes managing data is bad

#

and it should manage functionality

#

U agree with that

woeful pasture Nov 1, 2023, 1:52 AM

#

ofcourse

#

abstract classes exist to manage basic implementations not enforce and manage data

#

ofc this depends on how you define "manage data" abstract classes having some basic set of data they work with isn't inherently bad

#

I'd say its maybe a bit more nuanced than abstract classes managing any set of data is bad. Remember though I'm pretty new to programming (only 2 years with java) so there is always a chance my statement isn't fully accurate

round fossil Nov 1, 2023, 1:55 AM

#

skinny, have u ever looked at AbstractList, or AbstractMap

#

they are abstract classes, mostly for convenience if u wna implement a custom map or list

night knoll Nov 1, 2023, 1:56 AM

#

I looked at AbstractList @round fossil

#

and that's what they told me to look at and it's what I noticed

round fossil Nov 1, 2023, 1:57 AM

#

yeah, so it exposes like one abstract method to minimize the complexity

night knoll Nov 1, 2023, 1:57 AM

#

Wdym

#

which one I didn't see

#

It's just really confusing for me

round fossil Nov 1, 2023, 1:58 AM

#

if u try to extend AbstractList

#

u'll see there is only one method u have to override

night knoll Nov 1, 2023, 1:58 AM

#

Because I don't know how I can check if someone's boots is superman's boots in my item implementation

#

and that also means the character class will define the PDC itself

#

Which idk if that should be done in that class

sly jackal Nov 1, 2023, 7:06 AM

#

what else should define the pdc

night knoll Nov 2, 2023, 1:12 AM

#

sly jackal what else should define the pdc

Idk it's just that it seems kind of off

#

let's say I have something like

#

public ItemStack getBoots() {

    return new ItemBuilder(Material.LEATHER_BOOTS)
    .setPDCValue(key, "reference-name").build();

}```

#

Idk how to explain

night knoll Nov 2, 2023, 11:43 AM

#

Factory?

interface ArmorFactory {
  ItemStack head();
  ItemStack chest()
  ItemStack pants();
  ItemStack boots();
}

public class DiamondArmorFactory implements ArmorFactory {
  ItemStack head() { /* Returns new diamond helmet itemstack */ }
  ItemStack chest() { /* Returns new diamond chestplate itemstack */ }
  ItemStack pants() { /* Returns new diamond pants itemstack */ }
  ItemStack boots() { /* Returns new diamond boots itemstack */ }
}

pine grail Nov 2, 2023, 11:43 AM

#

factory

opaque plover Nov 2, 2023, 11:44 AM

#

Factories are insanely hot both literally and figuratively

#

If you have a factory in your code, you are one step closer to marrying me

night knoll Nov 2, 2023, 11:46 AM

#

No sorry.

opaque plover Nov 2, 2023, 11:48 AM

#

I don't think this was a question 👀

pine grail Nov 2, 2023, 12:08 PM

#

opaque plover If you have a factory in your code, you are one step closer to marrying me

Aight what's step 2

opaque plover Nov 2, 2023, 12:27 PM

#

Send me 17000$

jagged rock Nov 2, 2023, 12:28 PM

#

night knoll Factory? ```java interface ArmorFactory { ItemStack head(); ItemStack chest...

public interface EquipmentFactory {

  ItemStack create(EquipmentSlot slot);

}

opaque plover Nov 2, 2023, 12:29 PM

#

🤤

jagged rock Nov 2, 2023, 12:30 PM

#

public class ConfigurableEquipmentFactory implements EquipmentFactory {

  private final ConfigurationSection node;

  public ConfigurableEquipmentFactory(ConfigurationSection node) {
    this.node = node;
  }

  @Override
  public ItemStack create(EquipmentSlot slot) {
    String path = slot.name().toLowerCase(Locale.ROOT);
    ConfigurationSection itemSection = node.getConfigurationSection(path);

    if(itemSection == null) {
      return null;
    }

    return ItemBuilder.fromSection(itemSection); // replace with your own util
  }
}

#

is how I'd do it

opaque plover Nov 2, 2023, 12:30 PM

#

jagged rock ```java public class ConfigurableEquipmentFactory implements EquipmentFactory { ...

💍

night knoll Nov 2, 2023, 8:01 PM

#

the only problem I have rn is the namespaced key

#

i want an easy system to manage it while being abstract with it

#

because every item will use the same namespaced key like this

{"reference_name":"name"}```

woeful pasture Nov 2, 2023, 8:36 PM

#

@night knoll is your code on github?

night knoll Nov 2, 2023, 8:36 PM

#

woeful pasture <@456226577798135808> is your code on github?

yeah but it isn't updated there

woeful pasture Nov 2, 2023, 8:36 PM

#

push your changes and send me a link

night knoll Nov 2, 2023, 8:59 PM

#

woeful pasture push your changes and send me a link

i havent made any updates

#

im still tryna figure out how to do this

woeful pasture Nov 2, 2023, 8:59 PM

#

you can just make your key on the fly using the static methods from namespaced key

night knoll Nov 2, 2023, 9:01 PM

#

bruh

round fossil Nov 2, 2023, 9:05 PM

#

I do it without passing my plugin instance. altho thats generally considered a bad approeach, but that way I can create true constants so to speak

woeful pasture Nov 2, 2023, 9:09 PM

#

round fossil I do it without passing my plugin instance. altho thats generally considered a b...

I do to the plugin instance is just inconvient

#

you can always do static final fields with the plguin instance if you use static getter

round fossil Nov 2, 2023, 9:10 PM

#

woeful pasture you can always do static final fields with the plguin instance if you use static...

yeah, its just that its a bit ugly and depending on how u design it, it may prove inflexible

#

hence why I avoid it, altho for 99% of users, it wont matter :>

night knoll Nov 3, 2023, 1:05 AM

#

@round fossil

alpine anvil Nov 3, 2023, 1:05 AM

#

epic conclube mention

night knoll Nov 3, 2023, 1:05 AM

#

is it OK if I make an abstract class to help a functionality

alpine anvil Nov 3, 2023, 1:06 AM

#

what functionallity

night knoll Nov 3, 2023, 1:07 AM

#

like

public item getItem() {
return getItemBase().setPDCValue(a, a, a);
}

public abstract item getItemBase();

#

What does that mean

alpine anvil Nov 3, 2023, 1:12 AM

#

i would say if setting the pdc value generates the final item id call it someting like build() so you can do something like Armor#build()

night knoll Nov 3, 2023, 1:20 AM

#

The main issue I have is setting, then getting the correct PDC key in something like a damage event

#

and I don't wanna directly hardcode those values in

sly jackal Nov 3, 2023, 9:00 AM

#

then make a class that fills them in for you

night knoll Nov 3, 2023, 11:11 PM

#

is it ok to nest all that under item

#

or should I put them in a new package called "custom"

sly jackal Nov 3, 2023, 11:45 PM

#

I think this is fine

night knoll Nov 4, 2023, 2:02 AM

#

ok thx

spring reef Nov 4, 2023, 10:20 AM

#

night knoll or should I put them in a new package called "custom"

would be redundant

#

only extra package i would see there is for armor

night knoll Nov 4, 2023, 1:18 PM

#

@spring reef

#

wait nvm

night knoll Nov 7, 2023, 1:21 AM

#

https://github.com/skinnynoonie/CharacterWar review please, I tried to remake it according to some advice I received

GitHub

GitHub - skinnynoonie/CharacterWar: A Minecraft Bukkit plugin.

A Minecraft Bukkit plugin. . Contribute to skinnynoonie/CharacterWar development by creating an account on GitHub.

spring reef Nov 7, 2023, 7:43 AM

#

night knoll https://github.com/skinnynoonie/CharacterWar review please, I tried to remake it...

looks gtm on the first look. only thing i noticed are the single implementation interfaces which are redundant

night knoll Nov 7, 2023, 10:44 AM

#

Uh

#

I'm just trying to depend on abstractions

sly jackal Nov 7, 2023, 1:42 PM

#

but abstractions for no reason just makes things more complicated

night knoll Nov 7, 2023, 2:00 PM

#

spring reef looks gtm on the first look. only thing i noticed are the single implementation ...

You're talking about the repositories right

round fossil Nov 7, 2023, 2:02 PM

#

sly jackal but abstractions for no reason just makes things more complicated

Yeah there’s no such thing as a zero cost abstraction or whatever it goes like

night knoll Nov 7, 2023, 8:16 PM

#

idk but if I were to do this how i would like it

#

I would delete the CharacterInfo interface

#

And the CustomCharacter interface

#

And obv delete the repository interfaces

willow bone Nov 8, 2023, 9:27 AM

#

round fossil Yeah there’s no such thing as a zero cost abstraction or whatever it goes like

Rust moment

round fossil Nov 8, 2023, 1:14 PM

#

willow bone Rust moment

Not 0 cost and this is not the channel, I’ve already brought up this once with you

willow bone Nov 8, 2023, 1:17 PM

#

round fossil Not 0 cost and this is not the channel, I’ve already brought up this once with y...

hm?

#

also what do you mean by "I've already bought up this once with you"?

round fossil Nov 8, 2023, 1:17 PM

#

You went off topic in this channel before

night knoll Nov 8, 2023, 7:41 PM

#

I just kept my interfaces

#

Idk I liked having everything being an interface

round fossil Nov 8, 2023, 11:03 PM

#

night knoll Idk I liked having everything being an interface

thats good

night knoll Nov 8, 2023, 11:32 PM

#

round fossil thats good

well I got yelled at for having them

cobalt trellis Nov 9, 2023, 6:32 AM

#

Everyone thinks of stuff differently

round fossil Nov 9, 2023, 7:12 AM

#

night knoll well I got yelled at for having them

by whom?

jagged rock Nov 9, 2023, 7:38 AM

#

spring reef looks gtm on the first look. only thing i noticed are the single implementation ...

probably this

civic shell Nov 12, 2023, 4:20 AM

#

https://github.com/ToastArgumentative/ToastRPG

Just wanna know how it's lookin so far.

jagged rock Nov 12, 2023, 4:33 AM

#

It seems like you're trying to go into enterprise design before mastering the basics

#

The first two things I notice when I open your project are:

Static abuse
Not following naming conventions

civic shell Nov 12, 2023, 4:35 AM

#

wdym Static abuse?

#

naming conventions i get, i just can get into the habbit. they arent consistent at all.

#

I just made changes to the static methods.

jagged rock Nov 12, 2023, 4:43 AM

#

Still not good

#[MEGA THREAD] Get Your Code Reviewed or Review Others