[MEGA THREAD] Get Your Code Reviewed or Review Others

[night knoll]

and if they're missing values it uses fallback values

[woeful pasture]

like I if you use JSON configs and you want to ensure certain value types you can pretty much just error with explicit reasoning every time something goes wrong

for example if a config value is malformed I crash my plugin and give a detailed explanation of why

[night knoll]

yeah im too lazy for that

i just threw an exception that says what file is wrong

[rancid fern]

Watch your users not read that

Never trust a user to read anything

[night knoll]

not my job then 👍

but yeah I would like to do that

but how

[rancid fern]

get ready for a bunch of 1 star reviews

[night knoll]

like lets say the material

is incorrect

how would I let the user know

since it goes through the serialization and all those processes

[rancid fern]

Honestly handling the configuration directly ingame might be best

so the user doesn't mess up formatting

[night knoll]

ill prob do that

idk how "public" this plugin will be

[woeful pasture]

reminder to not dissalow out of game configuration though

those who know what they are doing tend to dislike in game configs

[rancid fern]

Yeah ofc

[woeful pasture]

got a couple friends who are experienced in server configuration etc and they despise in game configuration

[night knoll]

ima just hardcode everything

[woeful pasture]

I feel like that'd be a pretty common sentament

[night knoll]

so they can decompile and modifiy

[rancid fern]

same

Just from what I've seen some users struggle with the yaml format

and json might be harder

[night knoll]

lmao

I showed my cs teacher json in javascript

and she started to freak out like I was a genius

[rancid fern]

??

JSON is literally made for JavaScript

[night knoll]

IK LOL

shes like 65 tho

idk why she choose to teach js over python tho

everyone in the class constantly have errors with missing brackets

[rancid fern]

We're learning Java

Well it's the second time I have Java in school

First in high school now in Uni

[night knoll]

im only in principles

and the js we learn sucks so bad

we use code.org

and they have some crazy outdated modified js

like we dont even have lambdas

[rancid fern]

?

Why not let the browser handle the js

[night knoll]

code.org is out ide

our

[rancid fern]

🤷‍♂️ Never heard of it

thought it was a website?

The Java course I have now, is quite nice

We get automated JUnit tests for all our assignments and exams

[night knoll]

@woeful pasture how would I even get a detailed error message for this

like the most I can pin point is the file it is incorrect

[night knoll]

    /**
     * Loads a configuration with given fallback values. Should fail without throwing any exceptions.
     * @param fallback The fallback values to use in case of incorrect or missing values.
     * @return An empty Optional if this method failed.
     */
    public <C extends T> Optional<T> loadWithFallback(@NotNull C fallback) {
        Preconditions.checkNotNull(fallback, "Parameter fallback can not be null.");

        Class<? extends C> configClass = (Class<? extends C>) fallback.getClass();
        try {
            if (this.configService.isSaved(configClass)) {
                return Optional.of(this.configService.loadWithFallback(fallback));
            } else {
                return Optional.of(fallback);
            }
        } catch (Exception e) {
            this.pluginLogger.severe("An error occurred while loading a configuration! ("+e.getClass().getSimpleName()+")");
            this.pluginLogger.severe("Configuration name: " + this.getConfigName(configClass));

            if (e instanceof IOException) {
                this.pluginLogger.severe("Error message: " + e.getMessage());
                e.printStackTrace();
            } else if (e instanceof MalformedBodyException) {
                this.pluginLogger.severe("Error message: " + e.getMessage());
                if (e.getCause() != null) {
                    this.pluginLogger.severe("Error message (2): " + e.getCause().getMessage());
                }
            }

            return Optional.empty();
        }
    }

I know catching exception is usually bad but how would I print the first two error messages without repeating myself

[round fossil]

You know
try {

} catch (AlphaException e) {

} catch (BetaException e) {

}

is a thingv

But anyway one way is to extract a static helper method

Another way is to maybe make some methods to deal with specific exceptions in ur logger class

Also consider using Supplier<? extends FallbackType> since you may not want to eagerly access the fallback object

[night knoll]

does that mean I need to paste this:

this.pluginLogger.severe("An error occurred while loading a configuration! ("+e.getClass().getSimpleName()+")");
this.pluginLogger.severe("Configuration name: " + this.getConfigName(configClass));

maybe i can use a helper method for that

also im using bukkit's default plugin logger

this one java.util.logging.Logger

[night knoll]

    /**
     * Loads a configuration with given fallback values. Should fail without throwing any exceptions.
     * @param fallback The fallback values to use in case of incorrect or missing values.
     * @return An empty Optional if this method failed.
     */
    public <C extends T> Optional<T> loadWithFallback(@NotNull C fallback) {
        Preconditions.checkNotNull(fallback, "Parameter fallback can not be null.");

        Class<? extends C> configClass = (Class<? extends C>) fallback.getClass();
        try {
            if (this.configService.isSaved(configClass)) {
                return Optional.of(this.configService.loadWithFallback(fallback));
            } else {
                return Optional.of(fallback);
            }
        } catch (IOException ioe) {
            this.alertError(ioe, configClass);
            ioe.printStackTrace();
        } catch (MalformedBodyException mbe) {
            this.alertError(mbe, configClass);
            if (mbe.getCause() != null) {
                this.pluginLogger.severe("Error message (2): " + mbe.getCause().getMessage());
            }
            this.pluginLogger.severe("Please check to make sure there are formatting issues.");
        }

        return Optional.empty();
    }

this is what I have now

idk why exception handling does not look visually appealing at all

[covert marsh]

Why cant i call this in another class (even tho its static) and why does it not kick the player (even tho the method clearly says "KickThePlayer")

  private static strictfp void KickThePlayerFromTheServer(Player p) {
    for (Player p1 : Bukkit.getOnlinePlayers()) {
      if(p == p1) {
        Bukkit.getOnlinePlayers().remove(p);
        Bukkit.getOnlinePlayers().remove(p1);
      }
    }
  }

[jagged rock]

Are you testing other ppl or what?

[round fossil]

Roflmao

[novel meteor]

You forgot to check if p1 == 1 ofc smh my head

[covert marsh]

True... mb

[stone dagger]

jesus fuck probably the worst meme code ive seen in a while

[jagged rock]

bro got it from the hypixel code challenge

[pine grail]

this feels like actual code

you'd see

10/10 meme code

[sly jackal]

there are atleast 10 mistakes here, which is kinda impressive tbh

[stone dagger]

this entire code is a mistake

@sly jackal

[sly jackal]

oh right

11 then

[stark scarab]

you should check if p == Bukkit.getOnlinePlayers() first

[jagged rock]

use .equals so it compiles

[stone dagger]

Convert everything to a string and try equalsIgnoreCase

[stark scarab]

run javac inside a try/cache so that it definitely will compile

[stark scarab]

why is my paschulke plugin not working? https://paste.md-5.net/woniwoqoxi.paschulke

[covert marsh]

What even is that syntax. Is until a keyword in kt or is it a weird infix function?

[night knoll]

Too much functions

[sly jackal]

yeah it almost looks functional

[round fossil]

infix extension functions are interesting

[frosty grove]

love how that uses another vague .. operator in its implementation

[stark scarab]

yeah that's really cursed lol

[pine grail]

it means range

5..10 is an iterator from 5 to 10 (inclusive)

I used kotlin a tiny bit

.. == until but .. includes the last number where until doesn't

[frosty grove]

i know

i figured

but why not just use new Range(this, other - 1) or whatever in the implementation

[rancid fern]

because lazy

[stone dagger]

Man this Syntax is so ... different

It's same with Rust

[round fossil]

Well rust isn’t as bloated as kotlin with those qol semantics

[stone dagger]

Rust is still cryptic

And has a lot of syntactic sugar like traits

But it's at least const by default

Which I like

Rust = borrow checker simulator

[round fossil]

lol

[tawny tartan]

i honestly love how jetbrains did infix and inline functions

very useful

infix looks a bit weird but useful sometimes

[round fossil]

Yeah its a double edges sword tbh, not always the perfect semantic, but when used properly its nice

[covert marsh]

No its doomed. infix is one of the features i would completely block with sonar if i was to write a project in kotlin.

[pine grail]

infix can be useful if properly implemented and used, however I don't know a good usage of it

[round fossil]

operator overloading or like in general operations can help a lot

vectorA dotProduct vectorB

vs

vectorA.dotProduct(vectorB)

Essentially reducing actual boilerplate

[stone dagger]

ngl that just seems terrible to me, operator overloading is fine to me as long as used properly and actually overrides already existing operators

Like c++ basically, where you overload the arithmetic operators for matrix multiplications or similarly

But not if it's possible to overload an entire string

[round fossil]

why terrible?

Kotlin has infix + operator overloading

Every semantic can be used poorly, some semantics are harder than others, does that mean we shouldn’t use those? Well I don’t know, but I’d like to think there is a level of acceptance when it comes to getting those harder semantics fit right in your code.

[covert marsh]

I can get behind operator overloading as a language specification, but allowing any arbitrary symbols and words can lead to a whole meta-language. I had a lot of fun with infix functions but i would never use them in bigger projects.

[pine grail]

this is the only one I can agree with

turning some things into more english-like, i.e.

vectorA dot vectorB

[round fossil]

Nonetheless I think its partly up to the IDEs to paint extension, infix and inline methods in colors and text styles such that us, programmers, can understand which one is which

[covert marsh]

🖍️

[sly jackal]

yes but you shouldnt rely on syntax highlighting for readability

[frosty grove]

same

[frosty grove]

first one is the most ugly solution possible

[round fossil]

Yes and no

[round fossil]

how so?

[frosty grove]

imo at least

it just looks ugly

i cant explain it

id much rather call a method

[stone dagger]

Fair point, I mean it's the same with cpp where your operator overload can be used awfully in a semantic context. I guess I'm just not used to having entire strings have a different meaning through overload

[round fossil]

Well you’re still calling a method :^) but I get ur point

[stone dagger]

It's just so out of the usual syntax that it makes the code look wrong fr

I don't remember but there was a language where you were calling all methods like that

Don't remember the name

So weird

[novel meteor]

Small Brigadier command library for Spigot. 1.20+, ||reload "safe"||
Ignores bukkit permissions system (that's a feature)
Pls ignore the bracket style lol
Asking mainly for review of the main Brigit class ty 💚
https://github.com/steve6472/Brigit

[sly jackal]

sounds like something a functional language would try to do

[round fossil]

yep functional, or logical lol

[stone dagger]

Hello there,

so first of all I'd like to say that I despise curly braces on the next line. Second of all I would do these things a little bit (with that I mean a lot lol) different.

To actually give you some constructive criticism:

• You could replace Map<String, List<BrigitCommand>> with Guava's ListMultimap<String, BrigitCommand>
• you use the singleton pattern, yet you use many static methods for some reason?
• Your code is avoiding dependency inversion and interface segregation, you could try split things up into a main class, a Brigit Singleton that contains a repository of all commands and so forth
• Tabulator indenting?

and there's a couple things I would change overall. But if it works, it works!

I think @jagged rock has an entire post in #help-development explaining SOLID iirc

[jagged rock]

Regarding that first line, you should make a separate class instead of nesting collections

ListMultimap seems niche

For the second point, I'd replace it all with dependency injection

And creating new anon classes seems like a bad approach for commands

For commands I'd honestly go with something a bit more functional

Like wrapping brigadier around with a nice builder

[stone dagger]

Honestly it feels more like OP is still at the beginning, since they're only having little features yet

[jagged rock]

This is how I do my commands

[novel meteor]

Alright, so

• I am sorry you had to read the curly braces on next line :D it is more readable for me personally tho, so I'm keeping that one.
• I don't create new anon class for each command, but a normal class. One class per command. (I guess I could change the README.md)
• This is meant to resemble how Mojank does their commands but adds NMS -> Bukkit conversions (like Player and such)
• About the singleton, yep fair, but it is faster and "cleaner" to just call Brigadier.addCommandinstead of Brigadier.getInstance().addCommand
• About splitting it up, I can only think of splitting the Plugin and my Brigit logic
• I do not think I am avoiding dependency inversion ? (I probably don't understand it well enough)
• No clue how replacing everything with DI would even look
• Is tab indenting bad somehow ?

Ty for taking a look 💚

[novel meteor]

Hmm
The only other feature I would add is bukkit permissions :D
And maybe other util functions to BrigitCommand if I ever do need some.
(I just looked at your profile where it says your age, I'm mad, I'm older and I've been doing this for like 10 years by now and know so little :( )

[stone dagger]

I've been doing this for like 10 years as well haha

since I was little I started

[novel meteor]

Yeee same
On Minecraft modding :D

[round fossil]

no its good that u have it in a singleton

About the singleton, yep fair, but it is faster and "cleaner" to just call Brigadier.addCommandinstead of Brigadier.getInstance().addCommand
bad advice

[novel meteor]

Is it really that bad ?
Would the second way be preferred then ? Or is there other way ?

[round fossil]

addCommand being an instance method is better

that makes it way more robust in terms of design, testability etc

also more flexible

for instance if u wna consume it on a passed type paramter T

addCommand would imply u actually keep around a static map/list somewhere (ofc that method could just delegate to a singleton instance, but thats just awkward design)

[novel meteor]

I see, thank you, I will keep this in mind.

[night knoll]

Is it normal for a game plugin to be kind of coupled

[jagged rock]

yes and no

[stone dagger]

Hey there, static methods aren't necessarily faster. First of all even if they were, that's just a couple nanoseconds and counts as premature optimization. Second of all it is basically exactly the same. Difference being that you pass a valid "this" pointer to the function invocation under the hood. The JIT takes care of inlining when something is hot (called often), so even the method invocation (getInstance)isn't a big problem long term

[novel meteor]

I mean in terms of writing the code.
I don't really care that much about the speed :D

[stone dagger]

Yes there's some optimization certainly somewhere, but is it worth to even talk about?

[woeful pasture]

I agree with the cleaner call call :P generally I'd go with a fluid naming though and go with instance() that way it's shorter and still conveys its a singleton that has state

[stone dagger]

That's fine, it just sounded like you did

[stone dagger]

That's fine since you still utilize the singleton pattern. Tbh there are instances (pun intended) where using static to access something within the singleton is fine by me. If that's something immutable for example. Like a logger instance that is ensured to always be nonnull

[woeful pasture]

Loggers I usually just expose like willy nilly since like they're useful as fuck I don't wanna ahve to drag around my plugin instance

[stone dagger]

Instead of now having to write Plugin.getInstance().getLogger() you do Plugin.getLogger() // and semantically this is still valid

[jagged rock]

You could just pull a Bukkit

and make every static method redirect to your singleton

public static Logger getLogger() {
  return getInstance().getLogger();
}

[stark scarab]

glogger

it would be much funnier if it'd be called glogger and not logger

[stone dagger]

getGlogger

[stark scarab]

roast me:

Main class: https://github.com/mfnalex/ChestSort/blob/master/src/main/java/de/jeff_media/chestsort/ChestSortPlugin.java
Listener: https://github.com/mfnalex/ChestSort/blob/master/src/main/java/de/jeff_media/chestsort/listeners/ChestSortListener.java

(this was my first plugin ever and it grew very much over time, so it's a huge mess haha)

[stone dagger]

Damn single responsibility is something you really want avoided

Oh no

[stark scarab]

isn't that from isReadyToSort(...)?

oh no it isnt haha

yeah I should rewrite that whole thing

but I won't

because it works fine kek

[stone dagger]

¯_(ツ)_/¯

[sly jackal]

atleast it isnt nested

[round fossil]

yes but that is a pain to unit test

[stark scarab]

the whole code of chestsort is a pain

[restive ingot]

https://github.com/GospelBG/ChatPointsTTV there will be a lot of issues, this is my first plugin

[woeful pasture]

first thing I noticed

don't name your main class Main

https://www.spigotmc.org/threads/why-you-shouldnt-name-your-main-plugin-class-main.493289/

I'm not huge into twitch etc, but I feel like this maybe something that A. probably shouldn't be public or B. Should be provided by your plugin user
private final String ClientID = "1peexftcqommf5tf5pt74g7b3gyki3"; I could be wrong since I don't use twitch but generally I tend to see such ids kept private in envars

https://github.com/GospelBG/ChatPointsTTV/blob/master/src/main/java/com/gospelbg/chatpointsttv/CommandController.java#L40C1-L40C132 You shouldn't mix legacy chat and text components you can use ComponentBuilder to remedy this

[restive ingot]

I think that it wouldn't be a problem to be public. That is not an auth for my app, it is an id for it. You authentify to the app using your Twitch account via an OAuth connection

[restive ingot]

chatpointsttv.java may be a better name??

[woeful pasture]

if you're going to do this I highly reccomend setting up a proper singleton you can read about singleton pattern here While you can't do this perfectly with the JavaPlugin class it should give you a push in the right direction.

you don't need to do this check if your executor is in another class its just useless. That being the case you can probably get away with combining the sendLinkMessage method into the command executor

It might be wise to have a timeout for this task from what I see you just start the server every time their is a request and it seems like that async task never ends a malicious actor could probably crash a server by just spamming the link command and opening a ton of servers and asynchronous tasks

[woeful pasture]

yeah just make sure you follow java's naming conventions ofc so instead ChatPointsTTV.java

[restive ingot]

if you're going to do this I highly reccomend setting up a proper singleton you can read about singleton pattern here While you can't do this perfectly with the JavaPlugin class it should give you a push in the right direction.

I've just made a commit, is this what you were refering to? https://github.com/GospelBG/ChatPointsTTV/blob/master/src/main/java/com/gospelbg/chatpointsttv/ChatPointsTTV.java

[woeful pasture]

yes

[restive ingot]

How should I build it? Calling .build() causes me an error (The method build() is undefined for the type ComponentBuilder)

[woeful pasture]

yeah you gotta use #create right now

build doesn't exist in that version of bungeechat yet

unfortunately

[restive ingot]

For the timeout, do you mean to kill the task when a certain time passes or to rate limit the creation of a new task?

[woeful pasture]

well you need to end that task at some point what if they never click the link? Or something

just things you need to think about

[restive ingot]

Isn't that for component arrays?

[woeful pasture]

yeah you can only make component arrays atm

the BungeeChat version in spigot is a bit outdated, there are PRs in works to remedy that, but yeah unfortunately gotta cope atm

[restive ingot]

Oh, I understand now. I'll take it in mind for the next time

[pine grail]

this is fine, it's like discord client IDs

[restive ingot]

ok so i think i've solved everything you mentioned, check my latest commit and tell me what do you think

[restive ingot]

Thanks! I'll take a look at it tomorrow

Me neither haha I was rewriting that rn

Thanks for the tip!

[woeful pasture]

Hello Code Reviewers. Last Time I asked you to review this library it was apart of something bigger called Pineapple. Now I have removed it from Pineapple and it is its own library with multiple modules!!! I would love some feedback about what I, Y2K_ aka Miles_Dev could do to improve my amazing chat format!!! You may be asking what does said chat format look like? It's basically like mini message with a few differences you can find it's documentation here!
https://github.com/PineappleDevelopmentGroup/PineappleChat

[jagged rock]

Why does this feel incredibly non scalable

[woeful pasture]

because it isn't, but it is actually faster than the alternative atm

If more tag types are added I'd be more likely to go for a Map<Class, Consumer> approach

however with if statements atm their is no overhead of hashcode and equality checks. If java allowed pattern matching I could probably get it to compile done to a lookup table, but not possible in java 17

[jagged rock]

Given you already have a TokenType what's stopping you from making a getOffset method?

[woeful pasture]

I don't really understand what the benefit of this offset method would be?

[jagged rock]

Not hardcoding values here

[woeful pasture]

mmm true that's a good idea

[jagged rock]

You seem to be calling string.length() multiple times on Tokenizer

[woeful pasture]

can probably cache that huh

[jagged rock]

I wonder if you could use a switch for this

[woeful pasture]

I wonder too tbh, I was wondering if a switch would be worth it

@jagged rock much cleaner

    @NotNull
    public String detail(String source) {
        return source.substring(start + tokenType.getOffsetLeft(), end - tokenType.getOffsetRight());
    }```

[woeful pasture]

only problem with using a switch here would be the extra conditions that would need to be nested

[jagged rock]

You can write them branchless

that'd be cursed

[woeful pasture]

yeah probs

[jagged rock]

for(; cursor < length; ++cursor) {
  current = string.charAt(this.cursor);
  switch(current) {
    case TokenConstants.ESCAPE:
      escaped = !escaped;
      break;

    case TokenConstants.QUOTE_ESCAPE:
      quoteEscaped = quoteEscaped ^ !escaped; // branchless
      break;

    case TokenConstance.CLOSE:
      end = cursor += (1 & !(quoteEscaped || escaped));
      break;

    default:
      escaped = false;
  }
}

sumn like this

[woeful pasture]

what operator is this? is that xor?

[jagged rock]

yeah

[woeful pasture]

okay

[jagged rock]

I wonder if that's faster

try it

[woeful pasture]

I'd run a JMH, but can't atm on my laptop that would burn my batteryy

I'll check when I get home

my Tokenizer is actually pretty fast. It can do moderately large strings like
<yellow>random <gradient:red:blue:green><bold><obfuscated>traveler</gradient> <click:run_command:test command><underlined><red>click here</click><blue> to <rainbow><b>FEEL</rainbow> <obfuscated>it</obfuscated></blue> now things are going to get <rainbow>str<bold>a<italic>n<obfuscated>g</obfuscated></italic></bold>e</rainbow> <black>for now <gradient:#F0F00F:#0F0FF0>fairwell <bold><obfuscated>traveler</obfuscated></bold> in around 300ish nano seconds on my machine. I doubt the change would impact performance much

it does look nicer so i'll prob implement it anyways

[jagged rock]

you think that's large?

[woeful pasture]

no, not really its moderately sized for a minecraft chat

[jagged rock]

Here lemme give you a test case

the string I used for uh

:)

inv colors

<lang:space.18><lang:space.18><color:#ec1200>�<lang:space.1><color:#e21c00>�<lang:space.1><lang:space.18><lang:space.18><color:#c63800>�<lang:space.1><color:#bc4200>�<lang:space.1><lang:space.18><lang:space.-162><lang:space.18><color:#00a05e>�<lang:space.1><color:#009767>�<lang:space.1><lang:space.18><lang:space.18><color:#007a84>�<lang:space.1><color:#00718d>�<lang:space.1><lang:space.18><lang:space.18><lang:space.-162><color:#aa0055>�<lang:space.1><color:#b3004b>�<lang:space.1><lang:space.18><lang:space.18><color:#cf002f>�<lang:space.1><color:#d90025>�<lang:space.1><lang:space.18><lang:space.18><color:#f50009>�<lang:space.1><lang:space.-162><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.-162><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.-162><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.18><lang:space.-162>

have fun

actually I don't close tags there

[woeful pasture]

you don't need to with PineappleChat

lang is supported just fine

[jagged rock]

yuh

that's an old string

I remember having one that was like 1.5k characters long

[woeful pasture]

I once made a inv title so long it crashed my client

[jagged rock]

I've since optimized strings

[woeful pasture]

The slowest part of my parser is parsing my Syntax Tree into the actual chat format

[stone dagger]

@woeful pasture well returning in else if statements to get a fraction of a nanosecond better performance is definitely premature optimization. That's some next level premature optimization.

[woeful pasture]

Thx <3

I might honestly switch to using a map soonish tbh

That way it's easier to add tags

[viral mantle]

time to get my code reviewd

oh wow embed fail

anyway

main points to focus on are the API

and the game system which I used from @jagged rock.

and also the test implementation which isnt working in the WorldWarTNT-Plugin

(What's not working is that, SoldierGameJoinEvent fires BUT in the addSoldier method in Game class, it doesn't really add the player unless I do it two times?)

anyway would love to hear your guys' feedback on what I should improve on

[jagged rock]

I'd probably rename Soldier to User or Gamer or something

your isSoldier / removeSolider methods should allow me to pass a player for utility reasons

You could also make a game.getSoldier method that reroutes data

[viral mantle]

The reason for that is because in the name of the plugin well it has War

[jagged rock]

Game's event bus should be on the abstract game instead

[viral mantle]

But I get your suggestion and I will implement it

[jagged rock]

Can be lazily initialized

UUID should also be part of that

This should be configurable

So should your countdown stuff

[viral mantle]

wdym? how would I uh make this configurable? like make it abstract?

[jagged rock]

Basically each game should be able to be entirely configurable

Instead of hardcoding those values

You can have a game-specific config and a generic game config

On my engine I have a config for each map, and for each game a "game-config" and a "countdown-config"

the countdown config is for stuff like how many players we need to keep before cancelling countdowns

And messages at milestones

[viral mantle]

I see

[jagged rock]

And because this structure allows for multiple games to be ran on the same server you'll probably end up making something close to my "game template" class which loads the configs and creates the game when provided with a map

I'm not going to criticize the code itself but rather the misuse of the structure because I'd be biased

[viral mantle]

I see, Ill take your suggestions and ill try to implement them as best as I can

ill update you when Im finished

[viral mantle]

@jagged rock is it good practice to just have EventBus as a class and not as an interface? Because im pretty sure there is no other functionality that can be added that would give it purpose to make other classes implementing it or am I missing something?

[jagged rock]

Dependency inversion

[viral mantle]

Hmmm, so basically having the EventBus interface helps with more concrete code and reusability

Is having a GameSettings class a good idea?

I could store all the configurable stuff there

[viral mantle]

UPDATE for illusion @jagged rock

Rename Soldier to User ✅
Allow passing of player in isUser / removeUser methods ✅
Added game.getUser method that reroutes data ✅
Game's event bus should be on the abstract game instead ✅
Can be lazily initialized ✅
UUID should also be a part of that ✅
This should be configurable (GameMode) ✅
So should your countdown stuff (((( should be included in GameSettings class but I haven't added it yet ))))

https://github.com/YesNoBruhbruh/WorldWarTNT

embed fail 😭

[surreal peak]

useless check, remove only removes when theres something to remove

also make MySqlDatabase.connection lateinit

dont think you even need it nullable as it either gets initialized or an exception gets thrown

and i would make a dsl for that table builder

and whats this for? thats basically a copy of your already initialized field?

and store your users in a hashtable instead

[viral mantle]

whats that mean?

[surreal peak]

you know buildString {}, well that kind of syntax for a builder

buildTable(name) {
  row("id", DataType.Int) {
    nullable = true
    autoIncrement = true
  }
}```
something along those lines

[viral mantle]

I was trying to implement illusions suggestion of having the eventBus lazily initialized

[viral mantle]

is that for thread safety?

[viral mantle]

so to make the table builder easier to use?

[surreal peak]

no for more efficient lookup based on their uuid (i remember it was)

[viral mantle]

but we're looking for users, not uuids

[surreal peak]

did you remove the repo or smth

[viral mantle]

check again

also

@jagged rock sorry for the ping, Ive implemented your suggestions! Do you mins checking it out?

https://github.com/YesNoBruhbruh/WorldWarTNT

[jagged rock]

When you cancel the countdown phase you need to reset the timer

Messages can be cached on the constructor rather than on the start method

Split your logic into methods

Like these 2 should be their own methods

and the task just calls givePlayersWeapon()

But ideally you should have a kit system for this

Registering game states might be better to be done on the plugin class instead of the game class

But hey maybe your state registry is per game

Bit niche

Instead of a numerical ID your games can have a UUID or a 6 character string id

And I'd add a game join <id> command

Future improvements consist of making a kit system

You should also check and prevent spectators from damaging people

And maybe do a "vanilla mechanics" system to disable mechanics like crafting, hunger, opening chests etc

[surreal peak]

i would get rid of that isDatabase method, especially cuz it cannot be overridden (not an open method) and replace your map calls with putIfAbsent and remove it from your disconnectDatabase cuz youre already doing a safe call

[jagged rock]

hashmap wher

[pine grail]

hashmap my beloved

also isUser is redundant

you can just getUser and check for null in this case

[junior holly]

https://hastebin.skyra.pw/saduvenesi.java

If anyone would like to look through this player bounty plugin and give me suggestions or address some concerns perhaps? I know there's no gui listener, currently working on that class, other than that any notes or suggestions would be awesome.

[junior holly]

I hate the suspense haha

[cinder talon]

Overall, I would give a rating of 3.2 / 5.0 stars
(⭐⭐⭐)

1. Code Formatting

   • Some lines appear to have extra spaces or misaligned indentation. It's difficult to read. Remember to always format files, it can also be done automatically by your IDE

2. Unused Field:

   • The PlayerBounties class has a field private final PlayerBounties playerBountiesInstance; which is assigned to this in the constructor but never used elsewhere

3. Command Handling:

   • The BountyCommand class handles subcommands using if-else conditions. Consider using a switch statement for better readability and maintainability.

4. Magic Numbers:

   • In the BountyCommand class, there is a minimum bounty value (minBounty) that appears to be a magic number (100).
     You can define it with a static final variable to make it more clear

5. Configurability:

   • Some values like "bounties.dat" and "minBounty" are hard-coded.

6. JavaDocs:

   • Consider adding JavaDoc

7. Structure of code
   Code suffers a lot from absence of abstractionism, unnecessary nesting and low aesthetics of design.

Main class for instance has too many different aspects being tackled on the onEnable method, you should do huge steps separately and avoid having complex and modifying code in the enable phase.

9. Java 17 features
   In Bounty command and some other places you could really benefit from using modern Jav17 features.
   For instance you can avoid
   casting directly in a new line with the new instance of check assignment features
   if (sender instanceof Player player) { player....}

[jagged rock]

this mf thinking he smart

[junior holly]

Tbf to your comment about formatting, I like minimalized spacing a lot more

[prisma peak]

"code suffers a lot from absence of attraction"...?

[cinder talon]

Bro I type from phone

I meant abstract ion

🐎💨

[prisma peak]

I love my abstract ions

[cinder talon]

Android moment

[junior holly]

Well I like it, seems honest and genuine. I'm not a very creative person imo so when it comes to creative functionality I think I suffer a bit... That being said, I'd like to add more configurability such as the messages sent to the player or the formatting of lore for the heads displayed in /bounty gui etc.

[prisma peak]

passing the player bounties map around feels a bit bad, would be better to pass the plugin and add methods for adding/removing/etc

[junior holly]

Yeah I've also suffered with proper usage of these kind of things, I'm self taught in java through minecraft framework over the past 4 years, I'd say I have a solid year of spigot api experience

[cinder talon]

Is it a good practice to avoid nesting right ?

I mean you can do early return in your Interact Listener for instance (twice)

[jagged rock]

if you don't know you can't judge

Basically

Your map goes in a separate manager class

[junior holly]

I've heard so, although I feel like I've just done it too much over the years

[jagged rock]

Make separate methods for registering commands and listeners

[junior holly]

Don't do it in onenable?

[cinder talon]

No it's sad 😢 to do so much stuff on onable

[jagged rock]

you call those methods onEnable

Just split concerns a bit

[junior holly]

I suppose, then I initialize when I want / need?

Oh sorry, internet is slow didn't wait for you to say that

[cinder talon]

My onEnable on my premium quality software looks like this

[jagged rock]

@Override
public void onEnable() {
  this.manager = whatever;

  loadPlayerData();
  registerCommands();
  registerListeners();
}

...

[cinder talon]

It's a good aesthetics

[jagged rock]

congratulations it's still at the bottom of the class

it's ass

Splitting things up is cool but don't listen to cmarco's dellusion

[prisma peak]

Also this is just me hating on bukkit api but if you're making commands it's good to learn a command framework like ACF or cloud. Then you get proper suggestions and brigadier support, too.

[jagged rock]

I never managed to have cloud work

[junior holly]

Ok well, he gave me a 3.2 / 5 what would you give me? Be honest 😄

[jagged rock]

Make your own libs

[jagged rock]

Probably less

[junior holly]

Thats what I thought

[jagged rock]

I'm really strict

[junior holly]

You seem to know what you are doing so

[prisma peak]

Cloud 2 has better docs and is easier in terms of parsers imo

[jagged rock]

I already have a good enough command system

[junior holly]

Like I said, I'm self taught over 4 years only took stephan udemy course and tbf only put like 10 hours into that

[cinder talon]

I give extra score because I'm kind usually, I think 3.2 is ok but it can reach 4.0 easily

[jagged rock]

I'm self taught too

Started coding like 12 years ago, plugin dev for the past 7-8

Mindset matters if you want to help yourself and learn

For me to give a 3.2 it must be functional with some configuraibility

[junior holly]

Ah yeah I consider myself to have a years experience, the past four years or so it's been on and off so I just assume it adds up to about a year

[jagged rock]

For me to give a 4 it must be fully SOLID compliant and fully configurable

[junior holly]

That being said, I'd like to add more configurability such as the messages sent to the player or the formatting of lore for the heads displayed in /bounty gui etc.

[jagged rock]

For me to give a 5 it'd need to have full javadocs with examples, fully resilient, every argument must be checked and have error messages for everything

[junior holly]

What do you think of that type of config?

Or what else should be configurable?

[jagged rock]

Menu layout, menu items

[junior holly]

I'd love to practice config stuff, I haven't done much of that

[cinder talon]

If you want some tutoring service you can ask me by the way.
I feel like you have a lot of potential 🐎✨ to reach a 4.0 star quality in just a few weeks of training (⭐⭐⭐⭐)

[jagged rock]

shut up ai

[junior holly]

Ok well keep in mind, I suffered a traumatic brain injury last saturday and on 8 meds... I'm trying the best I can man

[jagged rock]

Yeah I'd recover from that before learning like a maniac

[cinder talon]

Yes I will pray for your my brother

😌🌇💞

[junior holly]

See I'm just trying to prove to myself that I'm still capable of this sort of mental process haha

[cinder talon]

By the way don't stress too much about code and aesthetics, it's a process

[jagged rock]

There's a lot of room for improvement but most of that improvement comes from experience

[junior holly]

Like not to bring up a sob story or anything but it was a near death experience and I just want to make sure since it was a head trauma, I'm still able to handle this kind of logic development. I just want to be "fine" more or less haha

[woeful pasture]

if you want some good advice about your code too my general advice here would be to ignore CMarco's delusions it should increase code quality

[jagged rock]

pov: you're insecure about your code

[cinder talon]

Can you stop bringing me down and dissing me?

[junior holly]

I'm also very young (17, 18 in march) so it's just the concern of cognitive ability I suppose

[jagged rock]

instead of settling for mediocrity why doesn't your stupid ass want to take some pride in yourself

but the good kind of pride

Not ego

[jagged rock]

because you're being beyond unhelpful?

[woeful pasture]

do you have github? I'd highly recommend posting code on there instead it becomes easier to navigate also you gain the positives of version control

[woeful pasture]

you are actively going aginst the point of this code review and giving horrid advice I don't mind if you meme around anywhere else, but its kinda messed up here

[jagged rock]

:(

[woeful pasture]

not talking to you :P

[jagged rock]

ah

[woeful pasture]

I have marco blocked

[cinder talon]

Me giving horrid advice?

Did you even read what I wrote, I spent so much writing it.

[junior holly]

I do yes, haven't used it in all of my 4 years of learning java however. (at least not for posting code myself, I only used it to look at people's source and api usage)

[woeful pasture]

I'd highly reccomend setting up version control with git

[jagged rock]

IntellIJ has github integration built in

[woeful pasture]

it gives the ability to have backups off disc too

[jagged rock]

You can just Git -> Share project on github

[junior holly]

Oh haha, I only thought I could download git projects

[jagged rock]

That's part of the fun

[junior holly]

Well last time I tried that, it was just a full kotlin plugin and I had absolutely no idea what to do with it haha

[cinder talon]

You can use kotlin for development

[junior holly]

Also how do I do this? I'm not sure where to find that

[cinder talon]

But remember to import kotlin stdlib if you make a kotlin plugin.

[junior holly]

I have never touched kotlin

[cinder talon]

It's kinda cool, I have one plugin published with it.

It's very null safe and optimised for fast writing

[junior holly]

Sounds pretty fun

[cinder talon]

By the way, can I send you a text I wrote about programming and aesthetics?

It will maybe motivate you and make you look at things differently

[prisma peak]

Personally I dislike kotlin specifically for bukkit development because it gets quite ugly due to how often nullable is used in the Bukkit API

[woeful pasture]

I highly reccomend you do not read his text about programming aesthetics

[cinder talon]

?paste

[rich fog]

https://paste.md-5.net/

[junior holly]

@jagged rock What do you think about this squaremap addon?

[jagged rock]

I hate it

[cinder talon]

Wow that is interesting

[junior holly]

It's not configurable... since it was specifically written for a person

[jagged rock]

There's no reason for your messages to be hardcoded

Or your materials

But also you should make a tick method and move your runnable logic there

Or ideally a separate class

Use guard clauses

[junior holly]

bukkit runnables are bad?

[jagged rock]

No clue why you're using an iterator do to over all online players

[cinder talon]

Lambda is a good addition to your code 😋

[jagged rock]

big lambdas bad

small lambdas ok

for(Player player : Bukkit.getOnlinePlayers()) {
  player...
}

This is fine for iterating over online players

No need for an iterator

[junior holly]

ah

Yeah that makes sense

[jagged rock]

Some variables can be cached

Like reusing player.getUniqueId() in the checkAndToggleVisibility

But also I'd like to see some separation of concerns

Split your permission checks from your logic

[junior holly]

Oh another thing about this, is that its kinda buggy when it comes to players trying to travel. If they need to eat food they risk the chance of being revealed on the map

[jagged rock]

your hidePlayer method shouldn't do permission checks

Your command should be the one doing that

[junior holly]

Haha that's funny

Yeah I wrote this one like 3 days after the accident

[jagged rock]

Messages should also not be sent in that method

Basically take "single responsibility" to the fullest extent

Not just for classes but for methods too

[junior holly]

Gotcha

[jagged rock]

a method to hide a player should only and exclusively hide a player

[junior holly]

That sounds awesome thank you

[jagged rock]

Not check if the player can be hidden or send a message

[junior holly]

Yeah that's correct for sure

[jagged rock]

Your onEnable method is responsible for scheduling a task, not defining its logic

type deal

Take this rule as far as possible and your code will end up quite a bit more organized

[cinder talon]

@junior holly

[junior holly]

Well I think the injury certainly has a lot to deal with my organization haha I've def been all over the place recently but I'll take note of that and start trying to stick to single functionality

[cinder talon]

Your Set<> field could be final I believe

To avoid accident reassignment

It doesn't bring any real performance boost, but it's usually providing a more elegant and safe code

[junior holly]

Fair point

What sort of mechanics would be fun for a mobcoins plugin? Currently I just have implemented a shop of sorts that will rotate the items every 12 hours (intention for a survival server so I'd want to implement config for the set of items that will be rotated through the shop (also the %chance for items to show up in shop should be configurable as well) and finally how often the shop will rotate)

https://hastebin.skyra.pw/obavaluvez.java

Also what do we think of this?

I believe this should be the same as that one, ran it through the jave formatter so I'm not sure

It's probably bad tbf I used a lot of static reference

[vast yew]

Hey would be very appreciative if anyone would review this project of mine. Its just a simple quest system. (Not finished but very close)
https://github.com/JuicySeals/QuestSystem

[night knoll]

Some basic things I noticed:

Format your code, make fields private, don't make a package called interfaces

[vast yew]

Can you explain more on what you mean by format it? All should be private where did I forget one?

[hidden maple]

there's one in CoinManager

[vast yew]

oh ok fixed

[woeful pasture]

you should use your plugin logger
https://github.com/JuicySeals/QuestSystem/blob/master/src/main/java/dev/blackgate/questsystem/util/Logger.java

public class MyLogger {

  private static final String PREFIX = "[Quests] ";
  private static Logger logger;

  public static void setLogger(Logger logger) {
    MyLogger.logger = logger;
  }

  public static void severe(String message) {
    Bukkit.getLogger().severe(PREFIX + message);
  }

  public static void info(String message) {
    Bukkit.getLogger().info(PREFIX + message);
  }

}

You can call setLogger in onEnable and supply it with your plugins logger

[hidden maple]

your first constructor in CoinManager should just take in a Database instance

that way you don't need two and it removes the 'for unit tests' thing

[vast yew]

👍

anything else

[woeful pasture]

Don't use Inventory Titles for these checks please
https://github.com/JuicySeals/QuestSystem/blob/master/src/main/java/dev/blackgate/questsystem/quest/creation/listeners/QuestCoinGuiListener.java

[night knoll]

Ah I didn't really look at all the classes but follow a consistent format

[woeful pasture]

I could easily break your plugin doing this good way to open up doops etc etc. Reminder that InventoryView#setTitle exists and can be manipulated by other plugins. You can also manipulate titles by renaming chests. I highly suggest you read this thread. https://www.spigotmc.org/threads/a-modern-approach-to-inventory-guis.594005/

[night knoll]

like this is what I do

public class MyClass {

    private final Object obj;
    private final String str;

    public MyClass(Object obj, String str) {
    }

    public void methodOne() {
    }

    public void methodTwo() {
    }

}```

and I checked ur class and you randomly had like 5 new lines

[vast yew]

Thats weird I haven't committed anything

[night knoll]

https://github.com/JuicySeals/QuestSystem/blob/master/src/main/java/dev/blackgate/questsystem/coin/CoinManager.java#L42-L45

I was looking at the mojang brigdaer whatever that's called

And their format is so consistent

[vast yew]

Earlier I was refactoring and didn't notice it

[night knoll]

Ye no worries

[vast yew]

thanks for all advice

[night knoll]

I think I might copy the Mojang format style or whatever their format style is

[woeful pasture]

brigadier is weird though you need to do some weird crap to get it to work on spigot

[night knoll]

idk I was just looking at their code

Just for fun

[woeful pasture]

it's not really brigs fault though its kinda spigots impl

the command API is like 13 billion years old

[night knoll]

Lmao

Hm

I also noticed that it doesn't really worry if a param is null

[woeful pasture]

I believe it just fails on nullability which is what all the good boys do

[junior holly]

Someome mind showing me how to upload a project to git with intellij?

[night knoll]

I just use github desktop

[woeful pasture]

#help-development probably better suits you there

[junior holly]

Yeah just realizedwhat chat I was in my bad

https://github.com/NukeCaps/EpirateTownyAddon/tree/master/src/main/java/nuclearkat/epiratetownyaddon

Could I perhaps get some feedback on this? The point of this plugin is to add a configurable cooldown time / messages to townJoin/townLeave events to prevent quick town hopping of sorts.

[hidden maple]

I might be wrong on this since I don't have much experience with multithreading.

I believe the getCoins method will return -1 if the executeQuery method executes the consumer on another thread since the getCoins method will return before the consumer executes.

That isn't a problem since executeQuery doesn't do that, but judging from the use of AtomicInteger, it might later on.

Also, you should be able to remove the array creation at the top of the getCoins method if you use one of the many provided List#of overloads that take in single elements.

[vast yew]

I have a unit test that tests that and it passes it so idk

Fixed the array creation thanks

[hidden maple]

It passes since executeQuery doesn't run the consumer on another thread.

I was saying that if it were to run on another thread I believe it would fail

[vast yew]

Oh I understand now

[hidden maple]

idk if that's in your future design but it's something to look out for

[junior holly]

https://github.com/NukeCaps/SilentSkyCore/tree/master

So this is something I have been working on for probably 2 months now? A couple issues: minion functions don't work also in the process of reworking them from zombie spawn egg to a neat looking armor stand, minions just kinda bugged haha. More issues just kinda relate to it being more so half written. One other issue with the island creation, is that for some reason it can't detect where the chest is and displays the error message associated with the chest inventory population function. Also still need to implement island permissions + ingame configurability

[novel meteor]

https://github.com/NukeCaps/SilentSkyCore/blob/14670381149750e82b15de7bcc133b7a6dc875e1/src/main/java/nuclearkat/silentskycore/islandcommandpackage/islandMinions/MinionPlacementListener.java#L33
Use early return, avoid nesting
Basically ```java
if (!(itemInHand.getType() == Material.ARMOR_STAND && itemInHand.hasItemMeta())) return;

For the `MinionEgg`, do not extend ItemStack and do not use Lore to store data, use PDC

https://github.com/NukeCaps/SilentSkyCore/blob/14670381149750e82b15de7bcc133b7a6dc875e1/src/main/java/nuclearkat/silentskycore/islandcommandpackage/AdminGuiCommand.java#L15
This one might throw error if ran from console, also, since you're using 1.20 you can use Java 17 (I believe) and its features
```java
if (sender instanceof Player player) {...}

[rich fog]

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

[novel meteor]

Playercoins I feel should not be stored in the command class
And the death listeners can be made in a better way, just make one class and pass the EntityType there

[viral mantle]

Thanks for all the suggestions! Last and most important thing, did I use your minigame structure right?

[viral mantle]

thanks for all the help dude appreciate it a ton!

[junior holly]

Hey thanks. Also I thought that project had my first “pdc” usage… I wasn’t really sure of how to use that concept. Other than that thank you for the feedback and I’ll sure be working on that

[stark scarab]

https://paste.md-5.net/arixuyubub.cpp << opinions?

[junior holly]

https://github.com/NukeCaps/EpirateTownyAddon/tree/master

Made some reformations to the code, was trying to implement some of the suggestions I got before about readability and strict single purpose / functions methods, also fixed an issue with me (stupidly) shading towny into the plugin, last thing I tried to do was reduce the strenuous error handling into a simpler system. Please let me know what you think and if I could make any more improvements!

[junior holly]

btw the sys.out messages are for debugging purposes

[junior holly]

New update is out, everything reformatted, commented, changes to the cooldown system (now using towny built in cooldonwns) that being said, removed the util class for handling cooldown data. Rewrote the event classes to handle the towny cooldown system. Finally cleared up a bit of error logging outputs.

Let me know of anything else that might be a concern!

[opaque plover]

Just a preference, but imo builders look better when their methods are as short as possible
e.g. withBuilderFace -> builderFace
And perhaps this could be a TagResolverFactory.Builder class instead of TagResolverFactoryBuilder?

[surreal peak]

use apply
instead of

fun withScheduler(scheduler: BasicsScheduler): TagResolverFactoryBuilder {
        this.scheduler = scheduler
        return this
}```
```kt
fun withScheduler(scheduler: BasicsScheduler) = apply { this.scheduler = scheduler }```

[sleek shard]

who puts comments below the code line 😭😭😭

[jagged rock]

I have him blocked so I never noticed that

[sleek shard]

valid af

[opaque plover]

I don't have him blocked just so that I could look at his messages and realize that I'm not as delusional and dum dum as some people

[junior holly]

#Inspiration

[frosty grove]

stfu cmarco is the best

[sleek shard]

ok

[junior holly]

https://github.com/NukeCaps/EpirateTownyAddon

Even newer update out. Every function is tested with the available configurability now added. It's pretty configurable along with being 100% tested and stable. I have made sure this whole project works for it's intended use and even tried breaking it a bit (still in that process), other than that I feel like it's pretty close to being finished.

The general idea is that this plugin adds a simple cooldown system to prevent townhopping on towny servers. The duration, messages, and functionality options are configurable. All in all I feel pretty good about this so far but any and all notes / suggestions are always something I look forward to reading.

[cinder talon]

Wow

functional programming + one listener

[jagged rock]

lmao I can get that done with like 5 lines of code

[junior holly]

"like anyone could even know that napoleon"

[viral mantle]

how

[cinder talon]

PluginManager.registerEvent()
Lol

[jagged rock]

sum reflection hackery

[viral mantle]

I meant like instead of manually doing all the events

[cinder talon]

Yeah

in my case. I could've just done a loop and use registerEvent()

At least for all the listeners functions that are equal to each other inside the method

[viral mantle]

👍

[cinder talon]

I would've just stored all the event classes I listen to in an enum, and looped over then.

I decided not to do it because on the future I might need to tweak each one individually

[viral mantle]

ye that sounds like a really bad idea

[cinder talon]

It doesn't bother me to have a 300 linea class tbh

So I'll leave it as is.

[viral mantle]

oh ok

[sly jackal]

as long as its readable/you can understand whats going on, the length of the code doesnt matter

[cinder talon]

❤️

Yeah I have some classes with 1200 lines and they're very organised

because of good design and aethetics, they can be read comfortably

[cobalt trellis]

I think my most organised 1.2k line class is https://github.com/Geolykt/EnchantmentsPlus/blob/4xx/src/main/java/de/geolykt/enchantments_plus/util/ColUtil.java, while my most disorganised class is https://github.com/Starloader-project/GslStarplane/blob/3e249afc622b40dd3f90edde5afe1d721188d40d/src/main/java/de/geolykt/starplane/Autodeobf.java (although in all it's fairness I don't think you can talk about organised code once it reaches 6.5k lines of code)

[surreal peak]

write those switch cases inline and you win 500 lines

[cobalt trellis]

Mind you that class was written in Java 8 days and hasn't been touched since because auto-refractoring that would require IJ which I am too lazy to boot up for that :p

[night knoll]

6.5k lines wtf

[cobalt trellis]

In all it's fairness it is the largest class I've ever written by far (the second largest being https://github.com/Starloader-project/Sl-deobf/blob/fd7125a28fec11552b50f6ab8a9417c85c1a9dc4/src/main/java/de/geolykt/starloader/deobf/Oaktree.java at 2.4k lines I reckon) and that mainly has to do with how I structure my ASM transformers as in aside from ASM there is 0 abstraction and everything generally tends to be located in static or static-esque methods which naturally invites monolithic design.

[opaque plover]

All my longest classes are util classes lmao

They get so large

[round fossil]

thats good

[sly jackal]

yeah thats because you have to put everything in a class in java

[round fossil]

yea but like, the concept of utility classes implies purity and statelessness which is gucci

[sly jackal]

yeah

without the class resriction it would be a file with a bunch of static like functions in a script if it was python or something

[round fossil]

lol yea

[opaque plover]

My kotlin utils class are funky because most of them are extension functions like

fun String.normalize(): String {}

and are file-scope (outside of a class)

[stark scarab]

What does it do?

[woeful pasture]

make it normal duh

it was weird before

[stark scarab]

ah yeah sure

[stark scarab]

here's my String.normalize method

[opaque plover]

Afaik lowercases and removes underscores and dashes

This function is for like some very specific tasks in my plugin, I don't even remember off the top of my head where I use it, but I do use it

[opaque plover]

This terrifies me

[stark scarab]

KNEF?

[opaque plover]

The whole thing

[junior holly]

https://hastebin.skyra.pw/zotodayice.java

public class VaultGUIUtil {

    public Inventory getVaultsInventory(Player player){return createVaultsInventory(player);}
    public List<String> getVaultsPermissionsList(){return vaultsPermissionsList;}
    private final List<String> vaultsPermissionsList = Arrays.asList(
            "rg.allowedvaults.one",
            "rg.allowedvaults.two",
            "rg.allowedvaults.three",
            "rg.allowedvaults.four",
            "rg.allowedvaults.five",
            "rg.allowedvaults.six",
            "rg.allowedvaults.seven"
    );

    private ItemStack createVaultItem(Material material, String displayName, int i) {
        ItemStack itemStack = new ItemStack(material);
        ItemMeta itemMeta = itemStack.getItemMeta();

        itemMeta.setDisplayName(ChatColor.AQUA + displayName.replace("%int%", String.valueOf(i)));
        List<String> itemLore = new ArrayList<>();
        itemLore.add(ChatColor.translateAlternateColorCodes('&', "&fClick here to access your&l&c #%vaultNumber% &fvault!".replace("%vaultNumber%", String.valueOf(i))));
        itemMeta.setLore(itemLore);
        itemStack.setItemMeta(itemMeta);

        return itemStack;
    }

    private Inventory createVaultsInventory(Player player){
        Inventory vaultsInventory = Bukkit.createInventory(player, 27, ChatColor.AQUA + "Vaults");

        int vaultSlot = 0;
        for (int i = 0; i < vaultsPermissionsList.size(); i++) {
            String permission = vaultsPermissionsList.get(i);
            if (player.hasPermission(permission)) {

                Material material = Material.ENDER_CHEST;
                String displayName = "Vault %int%";
                vaultSlot++;
                ItemStack vaultItem = createVaultItem(material, displayName, vaultSlot);

                vaultsInventory.setItem(i + 10, vaultItem);
            }
        }
        return vaultsInventory;
    }

}```
(other than no pdc how is this function?)

It's for a vaults gui where when the player opens it, it will display as many vaults as they have permissions

[stark scarab]

what's the purpose of getVaultsInventory if it just returns the result of createVaultsInventory?

ALso shouldn't a player have access to all 5 vaults, even if they only have permission rg.allowedvaults.five (and no permission for four, three, two, one)? Instead of only one?

Other than that, it looks fine

@junior holly

[junior holly]

To the first question, just messing around with some getter methods for some funky fun (also I just thought localization of function methods was a better practice),to your second question, I thought it to be a cool little thing that the vault options won’t even show up if you don’t have the permission

[stone dagger]

Instead of pre defining a list of permissions manually, I'd generate them using patterns. Just gives you much more options and is much more flexible

But if it works, it works

[round fossil]

until it doesn't

[stone dagger]

exactly

[junior holly]

That was also something I wanted to toy with, perhaps just configurable permission nodes in a sort of way would be fun

[tawny tartan]

been working on this project: https://github.com/InvoiceMC/Munch

I am currently working on a brand new serialization system since i dislike it rn but other than that how does it look, what should i improve?

[round fossil]

Async, but you join and await the thread anyways? (Looking at Functions.kt)

I’m not sure about that

[surreal peak]

whats the point of Munch::create if the constructor is public

[surreal peak]

uh oh no threadpool, that better doesnt get called often

[round fossil]

I’ll let you in on some insight, blocking is expensive, suppose the newly created thread of yours takes time to compute as it calls some database, then all threads that awaits the given thread may be suspended. Thread suspension is really expensive as it entails a lot of overhead, since it may have to deal with special JVM calls, OS-level and system level calls. If a lower priority thread is blocking a high priority thread you get what’s called the thread priority inversion problem. The higher the contention, the greater the ineffectiveness will be ntl. The entire point of being able to do this async would be to avoid having to await the result that is being computed in another thread.

[round fossil]

yuh

[tawny tartan]

Mmm, yeah for some reason I decided to do that instead of thread pools this one time. 99% of the time I do a ExecutorService.newSingleThreadExecutor() and go off that

[tawny tartan]

No point rlly I just like Munch.create(clazz) better, not sure why

I will fix the async stuff, ty ❤

[round fossil]

That’s not entirely a good choice either, it can heavily decrease service time of that single thread

Since u do io, u usually want amount of cpu threads * 2 + 1

or something

[tawny tartan]

to a fixed?

[round fossil]

fixed or cached

depends what exactly you wnna do

[tawny tartan]

how would i get the count?

Thread.activeCount() is what i found but i doubt that's the right one

[round fossil]

Runtime.getRuntime().availableProcessors()

[tawny tartan]

so just:

Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() * 2 + 1)```?

[round fossil]

(Usually we housekeep around n_cpu_threads + 1), but since you’re involving a lot of blocking by doing the io stuff, it reasonable to go beyond the classic amount

[round fossil]

Yeah, that probably works, you might want to have some custom thread pool executor, where the corepoolsize is the amount of available processors + 1 and the max pool size is double the amount

[tawny tartan]

👍 ty

[night knoll]

just make your constructor private

I can't rly read kotlin so idk if it's priv or not

[round fossil]

i mean it’s not necessary to enforce

Sure its a good defensive approach, but the static factory methods are primarily for long term maintainability and decoupling creation from instantiation

[night knoll]

yeah .create is kinda useless name lol

[round fossil]

if you still want to keep the property of subclassing for instance, it becomes reasonable to not internalize the constructor

[round fossil]

Its not

[night knoll]

I think it is if there's only 1 constructor

[round fossil]

nah

it doesn’t matter, static factory methods are almost always beneficial and superior to just explicit constructor invocation

[night knoll]

.forClass or something

[round fossil]

create is fine

create, valueOf, instance, <type>, of etc

all are fine

Its literally effective java item 1

[woeful pasture]

frrrrrrrrrr constructor invocations are for plebs

[cobalt trellis]

static methods everywhere 🌈 .
But you have a point - good APIs do not expose constructors - certainly not as-is. But for implementation-specific things? Everything goes.

[round fossil]

in multithreaded design it becomes crucial to avoid reference escape to ensure memory visibility, one good way to deal w that is precisely static factory methods

[tawny tartan]

👍

ty y'all ❤️

[tawny tartan]

after some benchmarks i concluded it runs WAY better than i thought it would imo, although i did some backend work today
process() = just processing annotations
generateTable() = Well pretty self explanatory but it generates the table SQL

[stark scarab]

thoughts about my "argument path" system? https://github.com/SpigotBasics/basics/blob/main/modules/basics-give/src/main/kotlin/com/github/spigotbasics/modules/basicsgive/BasicsGiveModule.kt#L57

[round fossil]

Looks like duct taped brigadier

[woeful pasture]

I wish brig wasn't aids to use with spigot

[round fossil]

i've been doing fine using it w spigot, I mean obv its not perfect

[woeful pasture]

Show me your brig impl I wasn't aware you could even fully use it without NMS or hacky l CB reflection

[round fossil]

i mean I have to use nms/cb ofc

but like, its still possible to make it doable

[woeful pasture]

show me your impl I'm curious

[round fossil]

ill ask my client rq, because this is code I write for a customer, otherwise Ill tape an example just for u

[woeful pasture]

should be fine as long as you aren't under NDA and it doesn't pertain specifically to their plugin implementation

I mean I looked at Command API and their implementation is wild as fuck

[round fossil]

yea

i havent looked at cmd api, but heard its messy

[stark scarab]

idk I never looked at brigadier, I only know people complain about it all the time

[round fossil]

yeah well, it gets verbose, and it is a bit of a hassle if u wna make it use CommandSender as its type S

i understand ppl who want something more concise or, well, annotation based

[woeful pasture]

annotations are 👎

but brig is very nice especially if you ever want to utilize the built in color awareness

[round fossil]

agree

and well, i like its design, tho thats more of a personal interest

[woeful pasture]

please get back to me on the impl if possible Conclube I wanna use it for my library, but I haven't beenable to think of a way to do it without meticulously going in and wrapping everything in bukkit types through pain and agony

[round fossil]

yea will do 👍

[woeful pasture]

I'd love to have something like this in spigot atleast exposed too

or do something with internals to make life easier

[woeful pasture]

looks pretty good

[night knoll]

@round fossil how much money do u usually get from commission

[sly jackal]

probably heavily depends on the amount of work xD

[round fossil]

dont think u wna know that

[night knoll]

nah im curious

is it like 20$?

[round fossil]

much above

unsure if im allowed to tell

[night knoll]

like an average lol

[round fossil]

well i get payed hourly

[night knoll]

how much

[alpine anvil]

knowing conclubes level of nerd its probably like over 20

[stone dagger]

Where you working

[jagged rock]

25 an hour?

[simple canyon]

First time I've done something like this, want to do it right. If you need some more info feel free to ask
||https://github.com/xceingSW/Skilltree/tree/master||

[novel meteor]

class Commands has wrong name, it is only one command
Player p = (Player)commandSender; Do not directly cast without first checking if commandSender is instanceof Player
for PerkEvents, follow java method naming convention
Same for Skills in PlayerSkillTree and the Perks package
I suggest renaming Skills to just Skill

Not sure about creating PlayerSkillTree every time you want to check for a skill
I suggest renaming "set0" to just resetSkill or something

[simple canyon]

yeah, all makes sense (my naming is generally pretty shit and inconsistant)
anything about the perk implementation specifically?

[novel meteor]

I'm trying to figure out why you've done this
The list seems kinda useless.

[novel meteor]

I can't really comment about implementation
Hopefully someone else can do that

[simple canyon]

I was about to write a whole thing about why I did everything and noticed i could just put p.mining(e) in the first for loop

[surreal peak]

please use dependency injection

[night knoll]

kinda a weird question

but if I design a library around Optional

should I force the library users to use the Optional

or add another method using Java's null

  Optional<Object> find(String key);
  Object get(String key);

instead of

  Optional<Object> get(String key);

or just let them use .orElse(null)

[round fossil]

ugh

I dont think its worth committing to javas optional class

like I've seen libs do that in the past, its at best meh in long term

[night knoll]

lmao ima just add both

[round fossil]

why

[night knoll]

cause i think im my lib there's def a use for the Optional

[round fossil]

such as`?

[night knoll]

there's a lot of internal mapping

[round fossil]

that doesn't imply inherent need for Optional

its just a null wrapper

if there would exist proper subtyping for Optional like
PresentOptional<V> extends Optional<V>
AbsentOptional extends Optional

then it would be much more powerful

[night knoll]

what would present and absent optionals have

[round fossil]

compile time semantics for whether a value is absent or present

[night knoll]

i dont get it

[round fossil]

lets say there is the scenario

class Map {
Optional<V> get(Optional<K> key) {

}
}

if we have map implementation wehre null values are not allowed, well here is a solution:

class WeirdMap extends Map {
@Override PresentOptional<V> get(Optional<K> key) {
}
}

[night knoll]

huh

[round fossil]

ofc we could just, well use @NotNull, but that thing isn't giving us the reassurance, and Optional<V> from Java cant express that in compile time, u'd have to evaluate it at runtime

[night knoll]

that just sounds like smthing to be documented

[round fossil]

ofc

but if you can write safe and readable and robust code

then why not choose that over commenting the code

[night knoll]

i guess that's fair

prob wont ever be implemented tho

[round fossil]

yea

but essentially

thats what the java Optional is missing

which makes it a weak candidate to just @zealous stoneable @chrome estuary

[night knoll]

bro rly pinged two people for that

[round fossil]

woopsie

[alpine anvil]

thats their fault

[night knoll]

💀

[round fossil]

but anyway, the java convention says to only use Optional in method return types

tho fuck the java convention

if u wna religiously commit to optional, just do it

[woeful pasture]

optional in method parameters is disgusting

java really needs a proper optional implementation for it to even be feesable

[round fossil]

yup

but well, its a thing in these functional java libs, so who am I to judge

[woeful pasture]

e.g. like you can do this in kotiln

when(optional) {
  is Some -> some.operation()
  is None -> defaultOperationMethod()
}

java pattern matching in 21 should make such an implementation possible, but in reality Optional is at its best when its simply not just any old Object

[night knoll]

lmfao

what if Java makes Object extend Optional

[round fossil]

built in nullability pretty much pattern matching

[night knoll]

that wouldn't even work

what am i saying 💀

[round fossil]

well

that would complicate the language SIGNIFICANTLY also

[night knoll]

what if

they made a special special feature

idk what im abt to say nvm

[round fossil]

wellk

valhalla does aim to enhance nullability

its promised we're gonna receive those jeps in the near future, so just pray lol

[woeful pasture]

what's it doing

to enhance nullability

[round fossil]

idr exactly but it has to do with primitive generics etc

[woeful pasture]

👀 that sounds beautiful

[stark scarab]

Updated my command thingy https://github.com/SpigotBasics/basics/blob/985b8b198ab040abb8b2829df782b1036bedd45b/modules/basics-give/src/main/kotlin/com/github/spigotbasics/modules/basicsgive/BasicsGiveModule.kt#L61

[dusty axle]

that’s a very intimidating water droplet

[pine grail]

looks like a nuclear droplet

[stark scarab]

well it's 3.3mb after all

[stone dagger]

It's Kotlin so I can't say much about it 🫠

But at least you know that now, very valuable information

[earnest cloud]

how can you hide the implementation of interfaces so that they don’t get in the way when writing code?

like bukkit

because I see the implementation class when I write code

or to hide you need 2 plugins where only api is used

[alpine anvil]

closing the package?

[earnest cloud]

I don’t know what exactly you mean, but if it helps hide the implementation then yes

[alpine anvil]

idk what you mean by hide the impl, but just press the arrow on the folder and you dont see it

[earnest cloud]

by hiding I mean literally hiding from the API user

[alpine anvil]

you would make seperate modules then

[earnest cloud]

just like in bukkit where the implementation is not visible

[alpine anvil]

the api gets published and the plugin shades the api

[earnest cloud]

and if the api is connected to bukkit api

[alpine anvil]

same thing

[earnest cloud]

how to separate modules

[alpine anvil]

maven or gradle

[earnest cloud]

i should have 2 plugins impl bukkit api 1) impl 2) api?

[alpine anvil]

no

that is very over the top for a spigot plugin

basically follow this except you only need 2 modules https://blog.jeff-media.com/maven-multi-module-setup-for-supporting-different-nms-versions/

[earnest cloud]

it turns out my API will use the bukkit API through the implementation of my API

[alpine anvil]

same concept applies

[wintry fern]

well bukkit doesn't hide implementation, its just the api is provided as a separate jar from the server for convience purposes. So how you would do it is you would have 2 modules. The first module is your implementation and its dependency is also your api and you would shade your api into your plugin. Then you just provide others with your api jar. But I mean unless you have a very large plugin and api generally no need to go through and separate. Could just create a package for all your api stuff

[alpine anvil]

you have an api module which does whatever and exposes api with bukkit api or whatever, you then implement your api in a plugin module that also use bukkit api

[earnest cloud]

separate pocket?

[alpine anvil]

modules is easier

by seperating the projects it makes it much harder for people contribute

also makes it harder for you to add features as it would require you to install, reload and then go instead of just adding a class and now you can implement it

[surreal peak]

https://github.com/FourteenBrush/MathExpressionParser/blob/master/core/src/main/java/me/fourteendoggo/mathexpressionparser/symbol/SymbolLookup.java

are the javadocs a bit understandable to read + any room for code improvements?

[junior holly]

I haven’t looked over it, but there’s always ways to improve the code. Someone will be sure to let you know

[woeful pasture]

[woeful pasture]

honestly this class you sent looks good. I think trying to do any more would be a completely uneeded optimization

everything is pretty clear

[surreal peak]

javadoc on indexLookup is pretty vague

[opaque plover]

why does this thread disappear magically even though i haven't left it

ok now it doesn't anymore lol

[alpine anvil]

it was closed/closing

[opaque plover]

Ah

[opaque plover]

don't let this thread die hangs on to it

[tidal basin]

Here I am!
https://github.com/Paulem79/FallingLeaves-spigot

[tidal basin]

If anyone can suggest me optimizations, I'll be glad!

[surreal peak]

dont use random instances on the go, they are not lightweight at all

[tidal basin]

the getInstance?

[surreal peak]

SafeRandom

[tidal basin]

oh yeah

[surreal peak]

also no method should ever throw an ArrayIndexOutOfBoundsException, that just means you're writing trash

[tidal basin]

😭

so, say that to 7smile7 x)

it's his method, but in savanna and taiga it throws this error

[surreal peak]

dont expose FallingLeaves.registeredLeaves.add(leaf)

[tidal basin]

what do you mean by expose?

[surreal peak]

showing internal details, if you ever decided adding a leaf should call some other method then have fun refactoring

[tidal basin]

humm

what should I do instead?

[surreal peak]

provide some addLeaf function or whatever

[tidal basin]

oh okay

[surreal peak]

also make FallingLeaves.leavesMaterials final and initialize it with Set.of, prefarably EnumSet.of but idk of Material is an enum anymore

[tidal basin]

but, It's for code longevity, not for optimization?

[tidal basin]

okay I do that

[surreal peak]

jit compiler would inline it anyways, so not talking about optimizations here

[tidal basin]

oh intersting

[surreal peak]

radius should also be final

[tidal basin]

do this really change something?

and is it also effective for local variable (variable in methods) ?

[surreal peak]

this whole bunch of static stuff actually, use a static block if you need

[surreal peak]

about what are we talking?

[tidal basin]

final

for variables

[tidal basin]

but, how can I access it then?

[surreal peak]

wdym access, FallingLeaves.STATIC_STUFF?

[surreal peak]

variables as in fields?

[tidal basin]

oh okay

[tidal basin]

public static vs public static final

what does in change for performances?

[surreal peak]

allows the (jit) compiler to do constant folding if it can prove it is/can never be changed at runtime

[tidal basin]

so it's better

[surreal peak]

ig

just makes you cant reassign it either

[tidal basin]

yeah

[surreal peak]

also make your runnables their own class, this just looks awful

[tidal basin]

x)

yeah

[surreal peak]

and we are not going to c# way are we?

[tidal basin]

what?

[surreal peak]

by putting an I in front of interface names

[tidal basin]

how can I change that?java public static Pair<NamespacedKey, PersistentDataType<Double, Double>> PDC_NEXTX; public static Pair<NamespacedKey, PersistentDataType<Double, Double>> PDC_NEXTZ; public static Pair<NamespacedKey, PersistentDataType<Byte, Boolean>> PDC_ISLEAF; { // Assign your instance to the field as soon as it's created PDC_NEXTX = new Pair<>(new NamespacedKey(this, "nextx"), PersistentDataType.DOUBLE); PDC_NEXTZ = new Pair<>(new NamespacedKey(this, "nextz"), PersistentDataType.DOUBLE); PDC_ISLEAF = new Pair<>(new NamespacedKey(this, "isLeaf"), PersistentDataType.BOOLEAN); }
I need to access my plugin instance

(for pdc)

[tidal basin]

So I'd better rename it Impl?

[surreal peak]

ah you need this too

wouldnt make them static in the first place, as theres only be going to be one plugin inst either way

[surreal peak]

idk id call it to what it provides, Colorable or smth, if thats even proper english

[tidal basin]

okay, because I saw that
https://stackoverflow.com/questions/541912/interface-naming-in-java/542007#542007

[surreal peak]

just dont prefix with an i

[tidal basin]

okay

[round fossil]

implements and extends got ur back 👀

[tidal basin]

x)

[junior holly]

https://github.com/NukeCaps/RefinedKits

You guys know the drill 😄 Notes / suggestions I'm all ears for, it's not done yet but uhhh getting there haha

There's some stuff that's not fully written... like I said wip

[pine grail]

IProvideNumbers

[wintry fern]

https://tenor.com/view/i-cant-give-you-numbers-i-cant-do-that-confidential-secret-ozark-gif-16280860

[spring reef]

we should make a code review software only for spigotmc

[pine grail]

"you called getInventoryContents twice MODS NUKE HIM"

[spring reef]

well, moderation-wise this could get difficult, but it would be a huge addition tbh

[round fossil]

If you need the functionality of RefinedKits java and not just JavaPlugin, its fine to directly dependency inject the plugin instance as type RefinedKits. However in a larger code base you’d ideally not want to have any external usage of RefinedKits. That is there should be no other class that needs to know your plugin instance of type RefinedKits, it should only be of type JavaPlugin. But for now don’t mind it.

Weird naming choice by having the interface named Kits and the implementation named Kit.

Could maybe use this keyword (imo helps w readability).

ConfigUtil is not really a utility class. The name is missleading since the class doesn’t conform to the pattern.

[junior holly]

You're awesome clube, for the javaplugin thing, I was just trying to make it a bit simpler of passing the main around where needed... also the naming of the interface / implementation comes from the fact I was cooked with a K, configUtil I guess you're correct, just rename/move to make it actually fit it's functions? ... After looking through it again, it's follows util principals but some things do not lol... I'll uh look into that for sure

[stone dagger]

Well it already kinda exists as linters, that evaluate code on a set of rules you can define

Linters can help

[opaque plover]

un-die this thread!! hanging on by a thread!!

[spring reef]

if nobody wanna sacrifice themselfs this will die again

[opaque plover]

i keep making noble sacrifices

[wintry fern]

https://tenor.com/view/star-trek-data-sacrifice-quest-in-a-quest-suck-as-ours-gif-16542820

[junior holly]

Too bad my projects aren’t really cool enough for this thread D:

[woeful pasture]

Not really spigot related in any capacity but curious on code thoughts 👀 https://github.com/Y2Kwastaken/CosmicTools

[jagged rock]

uH

You have a constants class yet have a bunch of hardcoded values elsewhere

You could make a constant out of the buffer size, urls etc

[woeful pasture]

true lol I should make that more consistent

I changed a lot when I moved to kotlin

[round fossil]

maybe add some coroutines to the downloading files part

idk

like maybe my advice is irrelevant, but if u get into kotlin, getting to know coroutines is useful

[opaque plover]

I suppose I want this reviewed? 👀
My first REST API with Ktor: https://github.com/RoughlyUnderscore/CubingHubBackend
Available at https://lapis-poppy3487.vm-host.com/api/v1/ for testing, implemented in CubingHub (which is right here: https://github.com/RoughlyUnderscore/CubingHub which you could also take a look at but no pressure 🤔)

Still waiting for comments 😛

[woeful pasture]

Ahh I wish I could but unfortunately flow matters and I need to await downloads anyways 🥲

[pine grail]

Yo cosmic reach tools already???

[alpine anvil]

He made that release day

[woeful pasture]

It's been out since day 1

[pine grail]

holy

[woeful pasture]

Its just like buildtools it's not complex :P

[alpine anvil]

Rip pink miles

[woeful pasture]

I'll give discord my money for the first time why not

[opaque plover]

According to my 🤓calculations, this thread will die in under an hour again. To resurrect it, please look at my code :3

[cobalt trellis]

https://github.com/stianloader/softmap/tree/main/src/main/java/org/stianloader/softmap is my failed attempt at writing a DSL, if one wishes to get a slight case of severe brain damage they could review that instead

Though https://github.com/stianloader/softmap/blob/16b2278ab2f83442af5c894f46492c62c8db56b4/src/main/java/org/stianloader/softmap/SimpleFramedRemapper.java#L135-L159 is probably the piece of code where I'd appreciate the most feedback since I've been writing methods similar to that one for far too often

I probably should use a LIFOQueue instead of a stock ArrayDequeue now that I think of it...

[surreal peak]

ayo exactly what i needed (i mean, some extra inspiration)

my attempt of dissecting a method descriptor was kinda awful

those frames, are those stack map frames or what are they?

[cobalt trellis]

For the framed remapper? They are just done so I can easily undo parts of the remapping requests as the way I implemented the DSL is rather brute-force

[surreal peak]

ah i had no idea what the frames are but i saw bytecode \🤓

[cobalt trellis]

The frames themselves have nothing to do with the bytecode

[surreal peak]

complex code heeheehee

[surreal peak]

and shouldnt this be "it can also be possible"?

[cobalt trellis]

The "be" is superfluous

Though it is likely that I need to scrap the entire project as differentials are hard

It certainly isn't the self-explanatory format I hoped it'd be

[surreal peak]

any recommendations?

https://github.com/FourteenBrush/MathExpressionParser/tree/d2cb80715f9f92cc39674a434559f3e21c3559d3/core/src/main/java/me/fourteendoggo/mathexpressionparser

[round fossil]

gradlify it

[surreal peak]

i will kotlinify it instead if you ask me

[round fossil]

lol sure

[surreal peak]

nah probably introduces extra overhead

[round fossil]

there is 1 thing I suppose

but its a nitpick

[surreal peak]

nitpicks are welcome

[round fossil]

close constructors of utility classes

and throw an assertion error inside of them

[surreal peak]

ah i deliberately dont do that cuz i tend to hold me to the rule of not introducing extra bloat without added value, might be a thing tho

[round fossil]

i generally agree w not bloating ur code

I mainly do it to stay consistent

[surreal peak]

if people see Utility and think heh lets do new Utility(), then can go fuck theirselves

[round fossil]

public final class Objects {
    private Objects() {
        throw new AssertionError("No java.util.Objects instances for you!");
    }

for example

java.util.Objects

[surreal peak]

ye sure

[round fossil]

also

// TODO: add support for removing/ inserting if absent

u pushed a todo comment to upstream?

[alpine anvil]

public final class EpicEbic {

    public EpicEbic() {
        System.exit(1);
    }
}

[surreal peak]

there are a lot

[round fossil]

oof, generally u dont wna do that since they usually fade into dont-dos

[surreal peak]

i mean how will i find them again if i remove the repo local

wouldnt be the first time i reinstall my os, i dont copy repos, i just make a [wip] commit and push it upstream

[round fossil]

well i mean u could just open issues or sth mean while, mostly just its somewhat of a practice to not check in todo comments

[surreal peak]

ah

[round fossil]

well im being nitpicky now, but yea lol

[surreal peak]

there are a few lol

[round fossil]

🥲

[surreal peak]

most of them are me not being able to deliver work on time

i definitely have some issues with scrum

[round fossil]

yea understandable

i hate scrum

[surreal peak]

scrum and agile are mostly nonsense to me

i cant remember the difference so i always mention both of them

[round fossil]

yeah, I can understand AGILE for long term

like if the end deadline is actually reasonable

[round fossil]

🥲

[surreal peak]

lemme find something

https://images-ext-1.discordapp.net/external/dHtRHXy6nHLVxTRNxtpPqxGEiDlEdokJrbD38vllkjc/https/i.imgur.com/3QQ7cfF.mp4

actual facts

especially the user stories and the "everyone starts doing random stuff"

[round fossil]

yea I mean much of the maintenance work is more like an investment long term

obv its not visible until months or likely years later

[surreal peak]

i think we achieved more in a three day hackaton with 5 than 6 weeks of working (6 hours a week) with a team of 9

its webdev, should mention that too, webdev is awful for systems programmers

like cmon i got better ways to waste my time than how do i get these buttons centered in a table row

anyways, that wasnt the subject

[surreal peak]

oh god the merge issues in college, i probably lost half of my code to it

[clever crag]

https://gitlab.com/mineson/mineson-api

[spring reef]

way too accurate

[hexed agate]

@ others keep in mind he is a beginner from his own admission so go easy

[surreal peak]

why is it called api when not an api

also use dependency injection