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

1 messages · Page 16 of 1

late smelt
#

I mean weak refrence exists for storing player objects

#

Doesn’t work super well with collections though

tidal basin
#

lot of idk lol

tidal basin
round fossil
late smelt
#

It won’t automatically be removed from the collection

#

But there are weak collections iirc which can handle that

round fossil
#

oh u mean like the weak-references themselves are lazily removed?

late smelt
#

Yeah

round fossil
#

yea, fair I mean I do think for example caffeine has sth for that, but yea, usually its not a big issue as it tries to get rid of stalled entries asap by polling the refqueue whenever possible

tiny sleet
#

I made the mace for 1.18.2

#

Aternos says I need popularity for this to be added to the plugins menu

rancid fern
#

Don't tell me you mean star reviews 💀

tiny sleet
rancid fern
#

we're not going to decompile it and give feedback on that

tiny sleet
rancid fern
#

Had a feeling that was the case

digital galleon
#

@tiny sleet do you give consent to decomp?

tiny sleet
#

Im new to this stuff

#

Sooo don't expect anything good

digital galleon
#

ReplIt AI built this yeah?

tiny sleet
digital galleon
#

Dont submit AI creations. Actually learn java

#

?learnjava

rich fogBOT
#

For Beginners:

Codecademy - Learn Java: Interactive Java programming course from basics to more advanced concepts. Perfect for absolute beginners.
https://www.codecademy.com/learn/learn-java
JetBrains Academy - Java Developer Track: Learn by doing with projects and challenges. It covers Java fundamentals to advanced topics.
https://www.jetbrains.com/academy/
Udemy - Java Programming Masterclass for Software Developers: Updated courses that cover Java 8 to Java 17 features. Suitable for those who prefer structured learning.
https://www.udemy.com/course/java-the-complete-java-developer-course/

For Intermediate to Advanced Learners:

Oracle Java Tutorials: The official guides by Oracle for Java programming—great for understanding the depth of Java.
https://docs.oracle.com/javase/tutorial/
Baeldung - Learn Java and Spring: Focus on Spring Framework and modern Java technologies. Best for intermediate learners aiming to expand their knowledge.
https://www.baeldung.com/

Practice and Hands-on Learning:

Exercism - Java Track: Solve exercises and get feedback from mentors. Great for practicing coding skills.
https://exercism.io/tracks/java
LeetCode: Practice your coding skills and prepare for technical interviews with Java.
https://leetcode.com/

Free Resources and Documentation:

Java Programming and Documentation: A comprehensive collection of Java programming guides, tutorials, and API documentation.
https://docs.oracle.com/en/java/

Community and Support:

Stack Overflow: A vast community of developers. Great for getting help with specific problems or understanding concepts.
https://stackoverflow.com/questions/tagged/java
r/learnjava on Reddit: Join the community of Java learners and get advice, share resources, and discuss projects.
https://www.reddit.com/r/learnjava/

Remember: Learning to program takes practice and patience. Don't hesitate to experiment with code and participate in community discussions. Happy coding! 🎉

digital galleon
#

@tiny sleet

tiny sleet
#

Well thanks for nothing 😒

solemn vine
#

💀

digital galleon
#

for nothing? Get a grip.

solemn vine
#

why are u making plugins again?

tiny sleet
digital galleon
#

damn I thought AI was better than this

solemn vine
#

messi>>>>

digital galleon
#

this is hideous

#
package com.mace.plugin.recipe;

import org.bukkit.Material;

class RecipeMaterials {
  final Material material;
  
  final Material blockMaterial;
  
  final Material handleMaterial;
  
  RecipeMaterials(Material material, Material blockMaterial, Material handleMaterial) {
    this.material = material;
    this.blockMaterial = blockMaterial;
    this.handleMaterial = handleMaterial;
  }
}
#

This whole class is redundant

solemn vine
#

OH 💀

tiny sleet
tiny sleet
digital galleon
#

Java 14 added record types. If that class is meant to be a record type, which im guessing it is, then its useless.

#

couldnt u do:

package com.mace.plugin.recipe;

import org.bukkit.Material;

record RecipeMaterials(Material material, Material blockMaterial, Material handleMaterial) {}
digital galleon
#

Learn to code please :D

rancid fern
#

It's vibe coded

#

They didn't even try

digital galleon
#

shitt forgot

#

he's nonchalant

solemn vine
#

im very chalant

tiny sleet
#

I really thought this server was help. I guess I couldn't have been more wrong 😒

jade delta
#

Are you asking for a code review or a resource review

tiny sleet
#

People just want to make Fun of my stuff.

jade delta
#

just to be clear

solemn vine
#

bro coded an ai plugin and wants us to give 5 stars

tiny sleet
#

I never said I want u to give it 5 stars

tiny sleet
rancid fern
#

Someone assumed you weren't begging for star reviews

#

and were asking for code feedback

tiny sleet
#

Well that seems like a personal issue

#

I guess I'm better off without this stupid server's "help"

solemn vine
#

?ban @ DrVoss promoting misinformation

#

happy now @ AstralVortxx

tiny sleet
#

Nah I'm out

jade delta
waxen tinsel
#

Hi, can I get some feedback on this project? 🙂

Its a plugin inspiered by the mod Immersive Energinnering. So each machine correspond to a block, each machine is called a "component" in the code. Several components that are connected with ropes (wires), form a network.

A network consists of suppliers and consumers.

  • Supplier, generates energy
  • Consumer, uses energy --> Machines

The energy to consumers is distributed as soon as a supplier receives energy. So if you only have one solar cell connected to the network, energy will not be distributed during the night.

round fossil
#

just because you use ConcurrentHashMap doesn't imply free atomicity

#

but also why use it there? in for example EnergyNetwork

waxen tinsel
sharp epoch
#

you can use github's own cache action for that, you could use git ls-remote in order to check whether a new version of spigot came and needs rebuild

round fossil
waxen tinsel
round fossil
#

to my second point, you should expose ur getter of that map as the return type of a ConcurrentMap if concurrency is a requirement

sharp epoch
#

I haven't checked the code yet, but if your operation is just being handled off-thread and not necessarily by multiple different threads, then you don't need it

round fossil
#

more so, its generally not a good idea to expose a collection itself (exposing a defensive view or defensive copy is often favored)

#

Optional.of runs at the risk of throwing

#

since between the containsKey call and get call, another thread couldve removed the entry for that key

#

causing get to yield null

waxen tinsel
round fossil
#

one way is to use the factory method ofNullable, but maybe there's a better design u could implement around

sharp epoch
#

if (components.stream().filter(component -> component.getUUID().equals(uuid)).findAny().isEmpty()) {
return Optional.empty();
}
this statement is redundant, findAny already returns an Optional that might be empty

sharp epoch
#

ah, you're checking against a components, but returning whatever componentConnections holds

waxen tinsel
round fossil
#

I'm not entirely sure about how you scale things wilmer, but you may consider using LinkedLists if you're adding and removing to the list a lot, tho if its supposed to support multi threading you'd need to use a thread safe collection

round fossil
#

Your read-write lock implementation in EnergyNetworkSerializer is completely flawed

sharp epoch
#

is there a particular reason you're separating each EnergyComponent in their own collection? I feel like that complicates things for no reason

round fossil
#

locking a lock doesn't prevent that lock from being removed from collections

#

a ReentrantLock (readwrite ones included) that is held by a thread can in fact still be garbage collected (additionally)

sharp epoch
waxen tinsel
# round fossil tbf, if u disallow V to be nullable, u can use a null check on ConcurrentMap#get...

Do you mean something like this?

    public Optional<List<UUID>> getComponentsConnections(UUID uuid) {
        if (components.stream().filter(component -> component.getUUID().equals(uuid)).findAny().isEmpty()) {
            return Optional.empty();
        }
        List<UUID> connections = componentsConnections.get(uuid);
        if (connections != null) {
            return Optional.of(connections);
        }
        List<UUID> newConnections = new ArrayList<>();
        componentsConnections.put(uuid, newConnections);
        return Optional.of(newConnections);
    }```
sharp epoch
#

instead of filtering and findAny, you could just use noneMatch

#

components.stream().noneMatch(component -> component.getUUID().equals(uuid))

round fossil
#

tbh why no just return componentsConnections.computeIfAbsent(uuid,_ -> new ArrayList<>());

#

then u get free atomicity also

round fossil
waxen tinsel
round fossil
#

fair, I should say if u do involve IO with async, please supply an executor to ur CompletableFuture instances

waxen tinsel
round fossil
#

atomicity means something happens as an indivisible action

#

other threads can either operate before or after that action, not in between

sharp epoch
#

what that means is that when doing:

List<UUID> newConnections = new ArrayList<>();
// action can happen here
componentsConnections.put(uuid, newConnections);
#

while in a different thread, while computeIfAbsent technically doesn't allow for that, as it is an atomic action

round fossil
#

oh final word, it can be quite nice to take advantage of ReentrantLocks fairness policy when dealing with IO

round fossil
sharp epoch
#

I personally don't understand that method to well either lol, I just picked it because the usage of filter and findAny annoyed meç

round fossil
#

🥲

sharp epoch
#

but the logic is that it checks whether there's a connection in the componentCollections CHM, if there isn't then it checks whether a component with that uuid exists and if it does, it creates an empty list as connection to that uuid

waxen tinsel
#

Will computeIfAbsent add the list to the map, or do I need to do that myself?

sharp epoch
#

it adds the list to the map

waxen tinsel
#

good

sharp epoch
waxen tinsel
#

no

#

thanks for saying that

sharp epoch
#

more reason to just unify these tbh

waxen tinsel
#

But do you belive its better to just have one list of components, and just get each type in the distributeRemainingEnergy & getRemainingEnergyConsumers methods?

sharp epoch
#

if by that you mean to just have one collection and then have what would be the equivalent of getConsumers and getSuppliers just be filtering methods, then yes that does make more sense to me

round fossil
sharp epoch
#

then again, you'd call the filtering every time you update the network so that may as wel not be the best option

waxen tinsel
#

becouse the filter methods might gonna call every tick

#

and it can exist a lot of networks

sharp epoch
#

yeah, I don't love it but it sounds sane enough

waxen tinsel
#

I do not love it either

sharp epoch
#

some javadocs with information about the whys on these decisions would be insightful not only for us reading the code but also for you in the future whenever you look that code, something to think about

sharp epoch
#

sure, that works, any documentation is better than no documentation

round fossil
#

my favorite phrase, and ill say it again, code reads more times than it writes, so be mindful of what u write

sharp epoch
#

one thing I do not like is how network updates are currently being handled though

#

you said it may be updated every tick, that means you're scheduling a run task later task every tick, which isn't good

#

you'd be filling up the scheduler queue that way

waxen tinsel
sharp epoch
#

you're better off having a single run task timer where you only call updateNetwork whenever a boolean is toggled via requestNetworkChange or whatever

waxen tinsel
sharp epoch
#

yeah, that is with the assumption your system does nothing some ticks

#

if your network thing indeed does something every tick, may as well just update the network every tick

tidal basin
sharp epoch
#
List<EnergySupplier> componentsToUpdate = components.stream()
                                        .filter(EnergySupplier.class:isInstance)
                                        .map(EnergySupplier.class::cast)
                                        .toList();
waxen tinsel
#

Thanks

sharp epoch
tidal basin
waxen tinsel
#

Do you mean for the components list in EnergyNetwork?

waxen tinsel
round fossil
#

i mean if u pop and push a list a lot

#

its generally better to go with a linked list

round fossil
round fossil
waxen tinsel
waxen tinsel
# round fossil read my messages above

If you mean "tbf, if u disallow V to be nullable, u can use a null check on ConcurrentMap#get to check if an entry exists for a certain key"

Didnt I fix that with this code?

    public Optional<List<UUID>> getComponentsConnections(UUID uuid) {
        if (components.stream().noneMatch(component -> component.getUUID().equals(uuid))) {
            return Optional.empty();
        }

        return Optional.of(componentsConnections.computeIfAbsent(uuid, k -> new ArrayList<>()));
    }```
round fossil
#

there are some problems with regional thread pinning for virtual threads in java 21

#

which is fixed in j24-25

round fossil
waxen tinsel
waxen tinsel
round fossil
#

i mean it depends, virtual threads w a semaphore can work, or a cached thread pool or fixed one

#

entirely depends on what u use it for etc

waxen tinsel
#

I go with an newCachedThreadPool then

sharp epoch
#

pinning shouldn't be an issue for you

#

I wouldn't worry about it, but if you care enough, just install java 24

waxen tinsel
#

okay

#

Should I have executorService just in the serializer class, or everywhere I use compleatublefuture?

round fossil
#

it can be fine to have a singleton for that executor and have some static helper factory method to construct those futures

#

up to u

waxen tinsel
#

Okay, I did make a commit now with the changes, was it something more that was wrong in the code that I didnt change in the right way, ex the serializer class maybe?

round fossil
#

ur serializer class remains broken

#

or the class that had those reentrant locks :>

waxen tinsel
round fossil
#

ur program may remove a lock from the map while its being acquired by some thread

waxen tinsel
#

do you mean

round fossil
#

no u cant just remove it

#

the only time you can realistically remove it is when no thread queries the lock, and no thread has the lock acquired

waxen tinsel
#

okay, so how could I fix it then?

round fossil
#

If ur entries dont scale horizontally and they have a session

#

like players

#

u can let the lock live for the life span of a player session

#

else u may wanna introduce an expiring cache

#

where if u query the lock, the entry is refreshed as it resets the expiration time

waxen tinsel
round fossil
#

it was an example

waxen tinsel
#

oh

#

so I need an expiration time then?

#

or might just one lock for everyhing be better?

round fossil
#

no then u'll spend more time waiting for the lock than ur program doing useful stuff

waxen tinsel
round fossil
#

as I said either u need some cache

#

that expires the locks after they havent been used for long enough

#

or if there's some invariant policy of when u can safely expire them

waxen tinsel
round fossil
#

I mean if it for example hasnt been queried for lets say 10 minutes, it gets removed

#

and mind u that removal is atomic

#

of course this implies u must not allow lock acquisitions for longer than or equal to 10 minutes, else u may face a problem ^^

waxen tinsel
round fossil
#

no so everytime u call locks.get(key) u reset the expiration

#

and by policy, u should be calling locks.get(key) whenever u require the lock instance

waxen tinsel
round fossil
#

remove should be atomic to get/computeIfAsbent

#

so remove either happens before computeIfAbsent

#

or computeIfAbsent happens before thus invalidating subsequent remove call

#

I personally use caffeine

round fossil
#

yeaa

#

Caffeine.newBuilder()
.expireAfterAccess(Duration.ofMinutes(10))
.scheduler(scheduler)

for example

#

u pass a scheduler to it if u want it to remove entries eagerly

#

else it will remove entries lazily

waxen tinsel
round fossil
#

its a replacement to concurrent hash map

#

LoadingCache<K, ReadWriteLock> = Caffeine.newBuilder()
.expireAfterAccess(Duration.ofMinutes(10))
.scheduler(scheduler)
.build(_ -> new ReentrantReadWriteLock(true));

for example ^^

waxen tinsel
round fossil
#

what I gave u is basically
Map<K,ReadWriteLock> = new ConcurrentHashMap<>();

#

but it has built-in expiration and load-if-absent policies

waxen tinsel
waxen tinsel
round fossil
#

ur using it wrong

#

look at the link i sent u

waxen tinsel
round fossil
#

ye sure

waxen tinsel
#

was it anything more that might be good to change?

#

You said that you wanted better and more java docs right?

round fossil
#

dont comment for the sake of commenting

#

thats useless

#

comment when something may need better explanation than what the code already provides

#

or if there is something the reader MUST KNOW when reading the code

waxen tinsel
#

yea, I have tried comment the most complex part of the code, but I known some places that might need some docs

alpine anvil
tidal basin
solemn vine
#

yo i need feedback on my db impl

alpine anvil
#

use prepared statements

solemn vine
#

i just learned without it sql injections are ez

dense leaf
desert ore
#

So unholy people don't want to touch it or something Idk

solemn vine
dense leaf
#

you nerd

digital galleon
# dense leaf you nerd

dont mean to offend you, but do you post this stuff to like show off? or like garner interests in your project rather than actually wanting code reviews?

round fossil
#

I do think he wants to get it reviewed

dense leaf
#

I just want feedback lmao

#

why else would I send it in the code review channel

sly jackal
#

the fact that every interface uses generic types is interesting atleast

#

but like whats the reason for all of it

#

I mean I could be dumb since I dont use Kotlin

#

but what does this mean kotlin public open class GameInstance<T : GameInstance<T>>( /** The context of this instance. */ public open val context: GameContext<T>, )

dense leaf
#

It means the context is a constructor parameter

dense leaf
#

and also wanna be consistent

#

if I use it in some places and don't in others, it's a bit annoying

sly jackal
#

well but I mean like why does the context need a generic of the instance

#

you know its gonna be a game instance anyways

dense leaf
#

because it needs the GameType<T> which has a game instance factory of T

sly jackal
#

yeah but like you made both of those things

dense leaf
#

hm?

sly jackal
#

well its like my argument now shifts to why does GameType have a generic (presumably) game instance

dense leaf
#

because the factory returns a GameInstance<T> so it's typed

round fossil
#

I think what steaf is really arguing about here is whether you actually need a system wide type construction

sly jackal
#

yes

#

and the answer to that either comes down to your preference I guess, or to kotlin

#

but I never used kotlin

#

so then I am to believe that its your preference

dense leaf
#

it's not a kotlin thing, in my case just a preference thing I would say

sly jackal
#

and then I wonder why you did it that way

dense leaf
#

I like not having to cast return types

dense leaf
sly jackal
#

yeah but in that case the method could just have a generic

#

instead of the whole class

dense leaf
#

it does - the generic just gets inferred from the game context's game type

#

so you're saying I have <T : GameInstance<T>> create(context: GameContext): T or something?

sly jackal
#

what does the def of BedwarsInstance look like

dense leaf
#

then I'd have to cast the factory which is mehh

sly jackal
#

yeah I guess its fine, its just bedwarsinstance plastered all over the place

dense leaf
#

fairs

sly jackal
#

but idk how you would possible change that

dense leaf
#

I don't think there's any good way to without doing a billion unchecked casts

sly jackal
#

I guess I would just use more of a compositional style compared to inheritance and generics

#

what does open mean in kotlin

dense leaf
#

means something can be extended (classes) or overriden (members)

#

everything is final by default

sly jackal
#

oh so without it you cant?

#

i see

dense leaf
#

correct

sly jackal
#

kinda like c++ then ig

dense leaf
#

I prefer that because people can't just randomly extend all your shit without it being intended

sly jackal
#

but with the actual enforcement probably

dense leaf
#

well, it throws a compiler error

sly jackal
#

yeah on c++ you'd enter the shadowrealm

dense leaf
#

there's a compiler plugin that lets you place an annotation on a class or a file to make everything open

sly jackal
#

yeah but that feels like its missing the point

dense leaf
#

yeah it's mostly for stuff like spring

sly jackal
#

you have to beg the question of why they add extra effort to make things open, and you probably means you dont wanna spam it everywhere, which makes me think more about the inheritance structure too

dense leaf
#

yeah that's exactly the point

#

also a question: is it fine to name generics stuff like this?

(Yes, I need the reified generics here to get the actual class of the phase)

sly jackal
#

what does reified mean

dense leaf
#

means you get the class of a generic

sly jackal
#

wdym exactly

hybrid osprey
#

java has type erasure

dense leaf
#

you can get the class of T, usually you have to pass it in

sly jackal
#

like if you make a waitingphase, wouldnt it just be BedwarsWaitingPhase<BedwarsGameInstance>

dense leaf
sly jackal
#

so here the names like Waiting and Round are purely used to specify which classes will be supplied? i.e. you have to now make the waitingphase supply the BedwarsWaitingPhase

round fossil
#

I mean I think you can design this state machine a bit more refined

#

cuz thats ultimately what mini game architectures end up like

#

whether u decide to populate it with generics or not

dense leaf
sly jackal
dense leaf
#

so I can get all three KClass (or Class) instances of them

#

I need them for debugging and filtering by type

#

otherwise I'd have the same one three times or so

round fossil
# dense leaf elaborate?

it feels like your design is suffering unnecessary type constructions just because u wanna feed in a type aware context in the beginning

dense leaf
#

do you mean I should use less generics?

round fossil
#

also ur error handling can be improved

sly jackal
#

it kinda explodes to the whole codebase

round fossil
#

less generics is the wrong emphasize

dense leaf
round fossil
#

Sort it into 2 categories, errors that are fatal, meaning the instance needs to transition into some sort of abortion phase, and errors that are recoverable, meaning you may have to transition into some intermediate phase to recover the error perhaps? Well in either case this is somewhat what I meant by refining ur state machine a bit, for example the transitions ^^

#

anyway i do think its a very good start and looks promising

dense leaf
#

the question is just - how do I determine whether an error is fatal?

round fossil
#

Of course you have to trust the implementer to throw the correct shit etc, but you can def incorporate support architecturally

#

whether u wna use error as values, throw errors or some other way is up to u

#

I should add on to this, some phases may have invariants guaranteeing them never to transition from or to an error phase etc, u decide rad but yea just to give u some ideas ^^

round fossil
# round fossil less generics is the wrong emphasize

I think less generics is the wrong word, just as long as its justified, its more so when u throw in a type construction because "this might come handy if this is implementation agnostic, I should make it type aware". You can consider design pov from outside and inside, maybe its advantageous for the implementer to be given proper type awareness, but from an outside perspective you may not necessarily wanna be forced to pass around type parameters left and right

dense leaf
#

that's fair

#

I'm probs gonna start by removing it from the world stuff since that's quite unnecessary and never even used

dense leaf
#

I'm a huge fan of errors-as-values in languages like Go and Rust but the JVM isn't made for it - especially with kotlin's unchecked exceptions

#

Arrow has some typed error handling I looked at which looks a bit painful to use tbh

dense leaf
dense leaf
rancid fern
#

You can use Try#of from the vavr library

#

if you want something more convenient

dense leaf
#

well kotlin has runCatching { ... }.getOrNull() for that stuff, it's still just annoying

rancid fern
#

fair enough

round fossil
#

I was more speaking on a larger scale tho

#

obv on the small scale of things, u still need to make a decision regarding that ^^

sharp epoch
#

if it is checked, it is a recoverable exception

dense leaf
#

well, kotlin has unchecked exceptions

#

should I just check for RuntimeExceptions

sharp epoch
#

rather, kotlin doesn't have checked exceptions, so yeah, a bit of an issue

sharp epoch
#

what people usually do to avoid having to depend on the user properly using your API is providing some kind of way to tell the game engine to error, which would essentially be something like Context#failed or Context#success (often times implicit)

#

where Context#failed throws the error you expect it to throw, and the context object itself hopefully contains enough information to recover from that state

round fossil
sharp epoch
#

Well don’t leave me hanging conclube

#

Why so, at least in Java that’s the norm

round fossil
#

Haha well I think its a bit too huge of a generalization to narrow it down to such principle

#

Like an invariant exception may be unchecked, yet it may be recoverable

sharp epoch
#

That I can agree with, mostly because in the plug-in development space people don’t know how to use exceptions at all

round fossil
#

oh yea, most definitely true

sharp epoch
#

But at the very least, in a closed system, it is a good rule of thumb to go by. As long as the design allows to impose it to an extent anyway

round fossil
#

Maybe, but in grand scheme of things you probably wanna recover from preconditional exceptions that often come in the form of unchecked. Yes its also for developers when they write incorrect prograsm that try to enter an invalid set of states (which ofc means compromising invariants). But often times you may find yourself wanna try to enter a valid state with some preconditional parameter, and if that fails (for example if the invariant is nonnegative values), you may wanna recover by graciously falling back to 0, whether u do that proactively or just with a try catch.

In a traditional sense, I personally like the philosphy of checked implying that something unpreventable may happen, for example the file u read may not exist, even to prior calls of checking if the file exists since u depend on some file api in java. Or opening a connection may or may not work depending if the url is valid or not, its unpreventable. However, depending on context both of these ops may be fully recoverable or not- unlike unchecked exceptions for preconditions (for example), where UUID::fromString can be prevented from throwing an error if u parse the string through some regex first for example.

#

And I say traditionally- because like, Java still has unchecked exceptions for unpreventable things, mainly when it gets to the functional side of things, such as throwing unchecked io exception if u stream the lines of a file through the nio api.

dense leaf
round fossil
#

yea good examples

#

depending on context, you may wanna recover, alternatively not

dense leaf
#

true I could add a coroutine context element for that, e.g. if an exception occurs in the tick loop, it dies, but not in an event or something

#

and then I can also have Exception whilst handling RadThinkEvent: IllegalStateException: No brain found

round fossil
#

coroutines work similar to goroutines right?

dense leaf
#

Yep

#

and their context elements are really cool

round fossil
#

cuz in java its a bit different since u often end up w completablefuture and u start calling back on eventual exceptions

#

very ugly way but ye

#

java is what it is

dense leaf
#

Yeah coroutines don't do that

#

but you can add coroutine exception handlers to the context

round fossil
#

ah right

#

yea sounds like it facilitates better design

dense leaf
#

and if it doesn't apply to your "filter" of some sort, you can just re-throw it

dense leaf
#

but that's also just a Job which can have completion callbacks and such

dense leaf
dense leaf
night knoll
#

CompletableFuture or 6mb jar bundle

round fossil
#

Icl tho, CF has so many dependencies as well, its not like its just one class, tho ofc jdk is always there so

dense leaf
dense leaf
# round fossil yea good examples
    val isRecoverable = exception is IllegalStateException ||
      exception is IllegalArgumentException ||
      exception is NullPointerException ||
      exception is UnsupportedOperationException ||
      exception is IndexOutOfBoundsException ||
      exception is ConcurrentModificationException ||
      exception is NoSuchElementException

I think this'll do

round fossil
#

Yea, you can also advocate for implementers and users to use some custom exception where u pass that as cause ^^

#

entirely up to u

junior holly
dense leaf
round fossil
#

but yea I do wanna mention rad, sometimes it might be the case that NPE is unrecoverable, i think its just so contextual

dense leaf
#

yeah I'm aware

#

in that case do proper null checks :p

round fossil
#

as example, but yea, u'd ofc have to assume that every transition from one state to another is valid, if u're given an invalid state, then ofc transitioning into a new state from that invalid state may result in another invalid state

dense leaf
#

what is state here

round fossil
#

well state as in consider ur program and all of its states

#

but for simplification lets say we have only 3 states, some player and maybe balance and the current game

sleek shard
#

wsg nerds

round fossil
#

first we may have an invariant like, balance should always be nonnegative, that ofc just means we need to program around that balance cannot be set negative, if that happens, u have to either recover and restore balance to be non negative, or u precautiosly validate before changing balance

we may also have some sort of acceptable transition, lets say player must be not null and current game must be null if we want to set current game to not null

#

lets also say in order to end the game, game must be not null and player must be not null

#

ofc if player is null and u end the game, ur transition is invalid (the start state was invalid) so u can expect errors etc

(here is where u may wanna allow another transition that deals w this as to make this transition valid/acceptable)

#

but practically it shouldnt happen

#

well if ur program is written nicely (obv this is all very theoretical, but yea)

sleek shard
#

coding go brt

dense leaf
alpine anvil
#

computer says no

jagged rock
#

I'd rather see more minimessage strings instead of whatever this component builder is

#

it just looks obnoxious and ends up taking a lot of space for a message

#

GameTypes found prob use some infix functions to make registering smoother

#

something like register(::TntTagInstance with GameTypeMetadata("axi:tnt_tag", "<red>TNT Tag</red>"))

#

type shit

dense leaf
#

I'll support mini but I really hate it tbh

sleek shard
#

string regex replacer >>> adventure replacer

rancid fern
#

Better than Kotlin DSL

dense leaf
#

I like my text builder

desert ore
#

Olivo is a real kotlin hater

sleek shard
#

idk why but adventure replacers are so slow

rancid fern
#

Yeah that's one downside of adventure and minimessage

#

They're quite slow

dense leaf
sleek shard
#

yeah literally serializing them back to a string and replacing them is about 20x faster

#

deadass

desert ore
rancid fern
#

I was actually working on that for a bit

#

Never ended up finishing anything though

desert ore
#

Damn

sleek shard
#

What I do for my custom replacer is deserialize a component to a string, and serialize it back to a component once replaced

#

and jesus it’s so much faster

dense leaf
#

or do I wanna do .mini, might be clearer

sleek shard
#

minimize string?

dense leaf
#

no

sleek shard
#

hahah

#

toMini()

dense leaf
#

meh

round fossil
#

I think paper uses the term rich right to refer to minimessage parsing?

sharp epoch
#

if you are going to use it, provide a language file instead

#

I do agree whatever that buildText is ends up being ugly though

#
buildText {
        append("TNT Tag")
        red()
      }

vs just

text("TNT Tag", RED)
hasty lotus
#

I made a very complex EventBus system, where I have interfaces for my events. I use ByteBuddy to generate concrete implementations for my interface events to pass to any subscribers which you can find here. It grabs the getter methods from the interfaces and creates the implementation class and load it into the ClassLoader. Then, I'm able to use a MethodHandle to create new event instances when I want to fire something to my subscribers.

I also use ClassGraph to find all of my event classes for ease here. It skips over any sealed classes because they are for parent events (for example, ArenaEvent) and searches for classes like ArenaCreationEvent and other "normal" event interfaces which can be constructed. Using a combination of both of these things, I construct a Map for my event bus, for which users can subscribe to a specific event like so.

// Subscribing to an Event
final Plugin plugin = ...;
final ApiEventBus bus = EventBusProvider.getBus();
bus.subscribe(plugin, ArenaCreationEvent.class, event -> System.out.println("Arena Created!"));

// Unsubscribing to an Event
bus.unsubscribe(plugin, ArenaCreationEvent.class);
// Or use bus.getSubscriptions(plugin, ArenaCreationEvent.class) or bus.getSubscriptions(plugin)
// and remove manually

Then to fire an event, I use the post method with arguments that are needed for the event

// Firing an event
bus.post(ArenaCreationEvent.class, arena);

All my event code is here. How should I approve upon this? I'm doing this EventBus system for fun btw

sharp epoch
#

I sometimes wonder if people are just scared to leave the plugin development space

#

these are things that could very well be done without any relation to minecraft or bukkit for that matter lol

hasty lotus
#

Yeah lmfao

late smelt
#

Pfft what else needs programmers these days

#

The AI’s handle it all!

round fossil
#

else epic

#

also maybe async api to some extent, not sure how relevant that is ^^

#

Oh nother thing, maybe u wna use an IdentityHashMap when K is Class? (you may also use Type instead of Class<T> and then invariantly cast; just to decrease type construction complexities etc)

round fossil
#

also

private final ListMultimap<Class<? extends MurderRunEvent>, EventSubscription<? extends MurderRunEvent>> subscriptions;
EventSubscription falls as a consumer or novariant, not producer - its possible contravariant, not covariant

#

yea ik java doesnt have declaration-site variance but still!!!

late smelt
#

Conclure once again saying words that make my brain confused

rancid fern
#

PECS

round fossil
#

^^

rancid fern
#

With fancy words

round fossil
#

producer extends, consumer supers

#
void method(Consumer<Object> in) {
  Consumer<? super Number> term = in; // makes sense
}
void method(Consumer<Integer> in) {
  Consumer<? extends Number> term = in; // makes no sense

  term.accept(Double.valueOf(1d));
}
method(o -> Integer.bitCount(o) /* not possible */);

void method(Supplier<Integer> in) {
  Supplier<? extends Number> term = in; // makes sense
}
void method(Supplier<Object> in) {
  Supplier<? super Number> term = in; // makes no sense
  Integer.bitCount(term.get()); // not possible
}
hasty lotus
#

Yes

#

private final ListMultimap<Class<? extends MurderRunEvent>, EventSubscription<? super MurderRunEvent>> subscriptions;

round fossil
round fossil
#

that pulse, and then u just have a Consumer<T> there since it will inherit the use-site contravariance (or not, i forgot u have Class<T> also)

hasty lotus
#

Actaully

round fossil
#

hii Pulse

hasty lotus
#

Could I use just ? instead of ? super MurderRunEvent

round fossil
#

myea

#

I do hate things that touch Class<T>

#

because Class<T> is invariant

hasty lotus
#

yeah

round fossil
#

since u have both cast() -> T and isInstance(T) so its not rly producer nor consumer

#

often times, I switch it out with Type, but that ofc comes with carefully and defensively designing around that you know its actually a Class<T> and only cast to Class<T> when necessary (on the use-site for example)

#

fuck java tbh

#

anyway, the eventbus looked good, id consider using it if it was a standalone lib completely genuine

hasty lotus
#

Hmm yes

#

And the sealed / non-sealed classes

#

does that logic work?

round fossil
#

is just ArenaEvent or LobbyEvent callable on its own?

hasty lotus
#

No

#

meant to be more like helper class

round fossil
#

so if I were to subscribe to LobbyEvent that'd yield an error, same to MurderRunEvent I assume?

hasty lotus
#

Yes

round fossil
#

its a bit unclear semantically

#

since u need to read code and write code under the assumption non-sealed means invokable

#

but also cancellation might be something u wna look into, else, handler ordering!

#

being able to (optionally) specify an order for an event subscription handler

hasty lotus
#

Hmmm

#

like a priority system maybe?

#

Similar to Bukkit

round fossil
#

yea

#

well I wouldnt limit it to like 5 tho

#

int limit?

hasty lotus
#

Yes

#

0 to int limit

round fossil
#

yea, wonderful

#

min val, 0 by default, max val

#

or the other way around ^^

round fossil
#

so maybe an annot to decide if an event is invokable?

hasty lotus
#

Yeah

round fossil
#

or rather, listenable?

#

all that aside, very great stuff

hasty lotus
#

i might as well port this to its own lib

round fossil
#

ofc that is if u dont mind

#

do u run checker fw btw pulse?

hasty lotus
#

Yeah I use checker

round fossil
#

pain

#

my dearest condolences for u, but also bless u

hasty lotus
#

checker gets in the way sometimes...

#

so when i get lazy

#

i just add a @SupressWarnings("all") // checker 💀

#

and i deal with it later

round fossil
#

HAHAHA

#

so feel u

hasty lotus
#

im too lazy to find the specific suppress warnings

#

like it just prints out especially for the big collections

#

like annotation spam

#

im like dealing with ts lmao

hasty lotus
round fossil
#

:,)

#

imagine java tail call optimization 🙏

hasty lotus
#

Yea i origianlyl wrote it into recursive method but i didnt just cuz

round fossil
#

yea no java is big troll when it comes to recursion

rancid fern
#

It's not that often you hit a stackoverflow when doing recursion and it being a stack limit rather than a bug

round fossil
#

true, i mean it is a shame u dont have tail call optimization tho- also to narrow down time complexity but also yea for the sake of the stack

rancid fern
#

Anyways I'm off to get some sleep, have a good night :)

round fossil
#

sleep well ^^

hasty lotus
#

@round fossil i added priorities, cancelling, and fixed the base event shit

#

And I also added an annotation called @NonInvokable

#

throws an error if I fire it

round fossil
#

looks nicee ^^

hasty lotus
#

now i have to make like

#

100 empty interfaces

#

and then add event calling

#

code

#

💀

round fossil
#

100?

#

ru coding 4d chess or what

hasty lotus
#

i just got have a lot of events to make

#

Actually

#

I could probably conjoin many events together

#

with some State enum or something

dense leaf
sharp epoch
#

you could just make your color registry extend RGBLike and go ballons

dense leaf
#

because then you need to actually specify the color registry entry instead of TextBuilder's red() which just looks it up from the registry

sharp epoch
#

do you have any example usage of your color registry? I need to see its utility before judging

dense leaf
#
public object Colors : ColorRegistry() {
  public val GREEN: TextColor = register(NamedTextColor.GREEN, 0x94f7c5)
  public val YELLOW: TextColor = register(NamedTextColor.YELLOW, 0xf6d48f)
  public val RED: TextColor = register(NamedTextColor.RED, 0xdd6777)
  public val BLUE: TextColor = register(NamedTextColor.BLUE, 0x86aaec)
}
#

and then you just call green() in TextBuilder and so on

#

here it's just registering with the ids of a named colour

sharp epoch
#

I don't understand why ColorRegistry needs to exist for this, are these colors used for anything else besides chat messages?

dense leaf
#

no, but they allow you to define the colours once and then use them everywhere

sharp epoch
#

like, it'd make more sense if the registry were to use theme-like concepts, say: main(), accent() and so on but I don't particularly see any reason to do green()vs Colors.GREEN given it is a TextColor

dense leaf
#

I do that in a different project and just find it annoying tbh

sharp epoch
#

though I kind of understand what you are going for here, I don't particularly see the utility to tie colors to some registry unless your library centers around the idea of establishing multiple themes for a singular game, which might make sense for some games and not so much for others

dense leaf
#

true I could make a theme system

dense leaf
alpine anvil
#

What about a language file

#

Wtf I'm blue

sleek shard
#

lolol

sharp epoch
#

I didn't think you'd take what I said and actually make a theme system lmao

dense leaf
sharp epoch
#

kotlin problems kek

dense leaf
#

but yeah I did just make a theme system

sharp epoch
#

well, if it works for you that's fine, just make sure it is abudantly clear of the purpose of the abstraction

#

I assume you'd want the users of this library or whatever to make use it for better integration or whatever, when you get to documenting it, try to make examples where it is in use so that anyone reading it feels more compelled to use it

dense leaf
#

good idea yea

hasty lotus
#

@round fossil i remember u were talking about FSM earlier

round fossil
#

yea

hasty lotus
#

I think my minigame is sorta an FSM? I have a start, end

#

And sort of update

#

And then status

round fossil
#

so u have states

hasty lotus
#

Yeah

#

i just dont formally defined them as that

#

Should I formally turn them into like defined states?

round fossil
#

i mean u have like

hasty lotus
#

and use some library to assist

round fossil
#

transitions also

hasty lotus
#

Yea

#

Would you use a lib like squirrel

#

or spring state machine to design

round fossil
#

ugh tbf I just end up designing it myself w a couple of sealed interfaces

#

not worth making any library for it

hasty lotus
#

Yeah true

#

I don't have many states tbh

#

so it isn't that bad

round fossil
#

and then yea, u need some state holder object usually, which ofc if u do need atomicity (lets say u wna go from one state to another atomically, compare-and-set), AtomicReference/AtomicMarkableReference/AtomicStampedReference etc ^^

hasty lotus
#

Is that like my GameStatus class?

#
public final class GameStatus {

  private final AtomicReference<Status> status;

  public GameStatus() {
    this.status = new AtomicReference<>(Status.NOT_STARTED);
    this.setStatus(Status.NOT_STARTED);
  }

  public Status getStatus() {
    return this.status.get();
  }

  public void setStatus(final Status status) {
    final ApiEventBus eventBus = EventBusProvider.getBus();
    eventBus.post(GameStatusEvent.class, this);
    this.status.set(status);
  }

  public enum Status {
    NOT_STARTED,
    SURVIVORS_RELEASED,
    KILLERS_RELEASED,
    FINISHED,
  }
}

round fossil
#

with events u'll usually have one major concerned set of states, that takes u from start state to finish state

round fossil
#

for example setStatus(doSomething(getStatus()))
doSomething(Status) -> Status

#

but obv this is just a concern in mulithreading, if multiple threads r allowed to mutate status

#

for example let's say 2 independent sources can trigger NOT_STARTED -> SURVIVORS_RELEASED

#

u obv (presumably) wna go from NOT_STARTED -> SURVIVORS_RELEASED exactly once

#

if (status.compareAndSet(NOT_STARTED, SURVIVORS_RELEASED)) {
//will only run once status is re-mutated back to NOT_STARTED
}

#

compare and set: swaps value of status if and only if current-value is NOT_STARTED, then new-value is SURVIVORS_RELEASED, this method is atomic so it means other threads see it as it either didnt happen or happen; that is one indivisible action ^^ (atomicity on hardware level)

#

(but yea if u sync status transitions on one single thread- then no need to care)

sharp epoch
#

don't use atomic wrappers for no reason, they're expensive

round fossil
#

eh not that huge of an issue

#

this is likely not gonna be a class with a billion instances

#

but if u do run into that issue, then obv VarHandle or AtomicReferenceFieldUpdater is the way to go

sharp epoch
#

reminds me when people started using AtomicInteger as counters inside runnables because it has that getAndAdd method

round fossil
#

yea, I mean tbf I think AtomicInteger now has plain methods

sharp epoch
#

like sure, that works but definitely not what you should be using it for lol

round fossil
#

with no memory effects

sharp epoch
round fossil
#

yea

sharp epoch
#

now you can choose between acquire/relase, opaque and volatile

round fossil
#

yea, I do think its odd still cuz like, half of VarHandle methods arent put in the atomic reference classes, and half of them are

sharp epoch
#

plain too, I just had missed it

round fossil
#

haha yea new stuff

#

like for example memory fences are still only through varhandle api by static calls

sharp epoch
#

it's nice it is available, but 99% of people don't even understand the different memory semantics so I doubt it gets much use

round fossil
#

ye true

#

and I mean, its only also to optimize if u truly need it

#

at times, code runs fine on volatile + some varhandle volatile CAS ops

sharp epoch
#

just let me manage memory with Unsafe#allocateMemory for maximum performance™

round fossil
#

:,)

#

another thing I personally hate are the "weak"/optimistic api from varhandle and also in the atomic reference api, like its in the api yet its one of those things that so depends on hardware and OS to some extent

#

since it can fail even if the CAS itself is succesful, but if given high memory contention

sharp epoch
#

I mean, hardware support shouldn't be an issue nowadays

#

I don't know of any modern CPU without atomic instructions at least lol

#

at least when it comes to compare and swap as well as fetch and add, which are the common ones, I am unsure of which instructions the weak/optimistic APIs make use of

round fossil
#

oh yea but that video doesnt go too indepth about it

sharp epoch
#

I still remember watching this video with the intent to play around with it but never actually having a project to put it in practice lol

round fossil
#

its one of those videos where u get familiarized w the topic, but its annoying since u kinda need to understand lower level programming to fully grasp the different "varhandle modes"

sharp epoch
round fossil
#

yea

sharp epoch
#

I was saying the exact same thing you just said but in different words lmao

round fossil
#

but yea I think the Javadocs are maybe just very poorly written

sharp epoch
#

though being completely fair, VarHandle and the like are very much out of the normal for Java APIs which tend to try and abstract away the memory concerns

sharp epoch
round fossil
#

cuz like weakCompareAndSet fails spuriously isnt something one understands until u've done concurrent and multithreaded programming in lower levels

sharp epoch
#

I hope they maintain the same level of care for the new vector APIs, I wanna play around with that

round fossil
#

🙏

#

it looked very good tho

sharp epoch
#

having to wait for valhalla is such a pain, and they're not getting any more feedback from it, it's just waiting now lol

#

inb4 just about valhalla gets released people are going to start questioning the implementation details

round fossil
#

yea

#

I mean Ido get that there are a ton of issues to resolve while keeping backwards compat

sharp epoch
#

I was really hoping they'd be done with it by java 25, that way everyone would jump on it as it's the next LTS

round fossil
#

for example union types tend to complicate type inference, adding null-restricted types must be done through an additive way etc

sharp epoch
#

but it doesn't look like to be the case

round fossil
#

yea rip

#

icl the issues of thread pinning vthread is nice that its being solved tho

sharp epoch
#

it's already been solved for the most part

#

I think there's one other case where pinning happens but it is much much rarer

round fossil
#

well most use j21

#

in where some major issues were still present

#

or should I say are present

sharp epoch
#

meanwhile me having to manually compile jetbrains runtime 24 😔

#

wish they'd just release binaries for non-lts

round fossil
#

🥹

#

lets see what j25 has to offer thus far

sharp epoch
#

they even have a 25 branch already, just not much done yet

round fossil
#
502:     Stable Values (Preview)
503:     Remove the 32-bit x86 Port
#

not too bad

#

structured concurrency api 🙏 pls no more previews

sharp epoch
#

Stable Values are good, no more having to use Suppliers#memoize from Guava

#

and it gets properly optimized so you don't have to worry about singletons not being able to be final anymore

round fossil
#

looks cool

#

ye huge

#

an L for 32 bit enjoyers tho- has to happen sooner or later tho

sharp epoch
#

bad time for 32bit enjoyers

pine grail
hasty lotus
#

You wouldn’t have support for base classes or any inheritance

#

You would have to manually do that for Proxy

round fossil
#

I do think Proxy might be the wrong tool

#

its a very minimal api

hard lantern
#

https://pastes.dev/J2EbA1f1qW
rabbitmq with kotlin, not sure if all the wrappers(in the channel class) for the interfaces are good or not. Also feels like im not kotlining enough with keywords and extension functions

sharp epoch
#

Did someone delete the messages

round fossil
#

me

late smelt
#

What did I miss :(

vagrant ether
#

I saw someone do one class per type but i was wondering if there was an easier way

dense leaf
#

what

random yacht
#

You started a question after skipping the prerequisite information lol

hybrid osprey
#

one class per type... i mean, yeah? that's a type

round fossil
#

1/1 = 1

#

🙏

alpine anvil
#

I'm guessing they mean an interface or abstract class with all the methods needed and an impl for each db type

sly jackal
#

doubt

dense leaf
dense leaf
#

like, why do you have your utils be classes and then sometimes put all the logic in companions

#

just put it at the top level..

ivory lotus
#

i want to learn but i really dont know if theres a better way

#

if you can give an example please 🙏

ivory lotus
alpine anvil
#

Use object xyz rather than a class with a companion object

#

You use it in some other places iirc

ivory lotus
dense leaf
#

objects are singletons

hasty lotus
#

Does anyone have a proper registry impl that I could take a look at

dense leaf
#

@latent thunder

#

oops wrong person sry

#

@junior holly

round fossil
#

Tho that is if u wna have a “freezable” design

hasty lotus
#

But as you can tell, it's pretty simple and ass lmao

round fossil
#

@SuppressWarnings("all") // checker
@hasty lotus i so feel u

dense leaf
hasty lotus
dense leaf
late smelt
#

But you said non-MIT compatible

#

Is MIT not compatible with MIT

wintry fern
#

but this is due to the fact it is the least restrictive

dense leaf
dense leaf
#

I had earlier said that "if a library is released under the GPL then any software which uses it has to be under the GPL itself

wintry fern
#

You will just end up in a scenario where you have dual licensing is all

#

However, one couldn't be more strict then what was already there

#

So, for example. MIT allows for straight copying of code as long as you give credit. GPL doesn't really cover this per-say and really aims at completed or whole works mostly, so therefore since gpl doesn't really say anything on it, and mit says its ok then its allowed for that portion of the works under MIT to be used however you see fit

#

however as for the other portions of code not under MIT you wouldn't really be able to do that

#

also the work that is licensed under MIT doesn't become GPL'ed either

#

because that would be you are removing a license and re-licensing which you are not allowed to do unless its your own works and you make clear that is what happened

#

reason MIT is compatible with GPL is the fact that MIT allows for open source viewing and modifications as well as distributing

#

which are all things GPL requires

sharp epoch
#

while you can have dual-licensed codebase, if you're using GPL in your code, then the binaries will end up being GPL even if the code isn't necessarily GPL as a whole

hasty lotus
#
    final UriSource source = UriSource.uri(URI.create("https://www.youtube.com/watch?v=dQw4w9WgXcQ"));
    final YTDLPParser parser = YTDLPParser.simple();
    final URLParseDump dump = parser.parse(source);
    final StrategySelector selector = StrategySelector.of(FormatStrategy.FIRST_AUDIO, FormatStrategy.FIRST_VIDEO);
    final UriSource videoFormat = selector.getVideoSource(dump).toUriSource();
    final UriSource audioFormat = selector.getAudioSource(dump).toUriSource();
    final VideoPlayer player = VideoPlayer.noOp();
    final AudioPipelineStep audioPipelineStep = AudioPipelineStep.NO_OP;
    final VideoPipelineStep videoPipelineStep = PipelineBuilder.video()
            .step(VideoFilter.INVERT)
            .step(VideoFilter.GRAYSCALE)
            .build();
    player.start(audioPipelineStep, videoPipelineStep, videoFormat, audioFormat);

yay or nay?

#

Making a video player lib and I liked the idea of a pipeline

wintry fern
# sharp epoch while you can have dual-licensed codebase, if you're using GPL in your code, the...

It depends. If the project is GPL and the libraries or the small additions are MIT, yes it would be overall GPL. If instead the project was MIT and the libraries are GPL then the main project wouldn't be subject to the GPL rather whatever is added to the libraries and that said libraries would be made available. However the main project would still be under MIT. The only things when it comes to using stuff that contains GPL code is that your project be open source. You wouldn't necessarily be required to make available your project rather whatever is added/modified to the GPL stuff. But you can have additions to a project that doesn't touch any of the GPL stuff and is simply utility in nature or stand alone.

round fossil
hasty lotus
#

is that bad

round fossil
#

tbh its really hard to tell how u designed this

#

since we just see some method calls and some variables

hasty lotus
#

hold on let me upload code

junior holly
dense leaf
#

that's his alt

sharp epoch
#

I don't see anything glaring from the code, after a quick look

#

power is using an AtomicDouble for some reason, are you accessing it on multiple threads?

junior holly
#

I believe in one or two places

#

Modifying it during a task at least

#

Probably would be fine without though to be fair

sharp epoch
#

I mean, unless you're using runTaskAsynchronously, that shouldn't be a problem

#

as for nitpicks, you have some duplicated code in the applyPhysics method, consider moving it to its own method

junior holly
#

Duplicated where? It's just the base of the flight physics

#

All the other physics methods are basically just modifications to applyPhysics

sharp epoch
#

the solid block collision and goal collision is essentially the same except one triggers an event it seems

#
            // Goal collision / flight end
            if (MathUtil.detectGoalCollision(player.getWorld(), discDisplay.getLocation(), currentVelocity) || tickCount[0] > maxTicks) {
                Vector throwDirection = initialVelocity.clone().normalize();
                Location teleportLocation = currentLocation.clone().subtract(throwDirection.multiply(1.5));

                player.teleport(teleportLocation);

                FFARound round = (FFARound) NDGManager.getInstance().getRoundHandler().getActiveRounds().get(0);
                Bukkit.getPluginManager().callEvent(new GoalScoreEvent(player, 1, round));
                if (this.discTask == null) {
                    discDisplay.remove();
                    return;
                }
                this.discTask.cancel();
                this.discTask = null;
                Bukkit.getScheduler().runTaskLater(plugin, discDisplay::remove, 100);
                return;
            }

            // Solid block collisions
            if (MathUtil.detectCollision(player.getWorld(), discDisplay.getLocation(), currentVelocity) || tickCount[0] > maxTicks) {
                Vector throwDirection = initialVelocity.clone().normalize();
                Location teleportLocation = currentLocation.clone().subtract(throwDirection.multiply(1.5));

                player.teleport(teleportLocation);

                if (this.discTask == null) {
                    discDisplay.remove();
                    return;
                }

                this.discTask.cancel();
                this.discTask = null;
                Bukkit.getScheduler().runTaskLater(plugin, discDisplay::remove, 100);
                return;
            }

            tickCount[0]++;

could be turned into:

#

well fuck you discord

#
private void handleFlightEnd(Player player, ItemDisplay discDisplay, Vector initialVelocity, Location currentLocation, boolean goalScored) {
    Vector throwDirection = initialVelocity.clone().normalize();
    Location teleportLocation = currentLocation.clone().subtract(throwDirection.multiply(1.5));

    player.teleport(teleportLocation);

    if (goalScored) {
        FFARound round = (FFARound) NDGManager.getInstance().getRoundHandler().getActiveRounds().get(0);
        Bukkit.getPluginManager().callEvent(new GoalScoreEvent(player, 1, round));
    }

    if (this.discTask != null) {
        this.discTask.cancel();
        this.discTask = null;
    }
    Bukkit.getScheduler().runTaskLater(plugin, discDisplay::remove, 100);
}

junior holly
#

yea that's nicer

sharp epoch
#

just a lot of params which is annoying but welp

#

another nitpick might be taking all the magic numbers around and turning them into constants, since you already have a Constants class

#

like that 220 for the gravity factor or the 1.5 multiplier

junior holly
#

Yeah I have yet to do that, but a lot of those values are still being tested

sharp epoch
#

other than that, I don't really have much to say about the code, the structure looks good so far

junior holly
#

I've been working on the physics for like 4 months kek, trying to emulate irl

#
public static boolean detectCollision(World world, Location startLocation, Vector direction) {
        double maxDistance = direction.length();
        RayTraceResult result = world.rayTraceBlocks(startLocation, startLocation.clone().add(direction.normalize()).toVector(), maxDistance, FluidCollisionMode.ALWAYS, true);

        if (result == null) {
            return false;
        }

        return result.getHitBlock() != null && result.getHitBlock().getType().isSolid() || result.getHitBlock().getType() == Material.WATER;
    }

    public static boolean detectGoalCollision(World world, Location startLocation, Vector direction) {
        double maxDistance = direction.length();
        RayTraceResult result = world.rayTraceBlocks(startLocation, startLocation.clone().add(direction.normalize()).toVector(), maxDistance, FluidCollisionMode.ALWAYS, true);
        if (result == null) {
            return false;
        }
        return result.getHitBlock() != null && result.getHitBlock().getType() == Material.CHAIN || result.getHitBlock().getType() == Material.OAK_SLAB;
    }```

Sophisticated ray tracing... wanted to rewrite this to detect like region collisions rather than specific blocks but eh eventually
sharp epoch
#

I appreciate the fact that you sent a manageable amount of code to review if anything

#

most people here send whole repositories and that's often too much for my lazy ass to manage a review lol

junior holly
#

I mean I finally got to a point where I felt like the flight looked good so posted that bit here, I was working on course gen stuff a while back but that's not going too well... gonna just do premade courses + we pastes for initial release until I've learned more about wfc / a* / etc

#

So maybe in another 4 months you'll get to review that kek

late smelt
alpine anvil
#

wait till you find out my alt is called femboyyy_ now

late smelt
#

I feel like I already knew that

junior holly
wintry fern
junior holly
#

I feel like the obstacle offsets need to be randomized a bit more, it still gens in rather straight lines

junior holly
#

I also figure I can integrate the obstacle gen with the chunk generator, just haven't looked into that enough yet

upbeat sinew
#

Hello!
I recently started making a quick utility library I made for myself to simplify command creation in Spigot.
I've gotten to a point of releasing it, so I wanted to ask any plugin developers - or just developers in general, to take a look at the features of it, and give improvement recommendations / feature ideas.
Thanks for anyone that can help!

You can see some previews of how it works in the two screenshots I attached.

To try it out, add these to your build.gradle:

repositories {
    maven {
        name = "commander"
        url = "https://dl.cloudsmith.io/qshTFUucaaD2Gctc/lilbrocodes/commander/maven/"
    }
}

dependencies {
    implementation "org.lilbrocodes:commander:1.0"
}
random yacht
#

What's the reason for passing a string for the plugin name when you have a plugin instance right there?

upbeat sinew
#

So you can do formatting on the plugin name used for the text in the brackets

random yacht
#

More prone to mistakes too imo

#

Could pass in the Plugin instance, then an optional name formatter string too if you wanted. i.e.

var root = new ParentExecutorNode(plugin, "automodpack", "Root command");
var formattedRoot = new ParentExecutorNode(plugin, "automodpack", "Root command", "&a%s");
#

Though I think the fact it's part of the executor node at all is a bit repetitive. Feels as though only the root node should have that as some sort of context, because otherwise you're in a position where you have to continuously pass the same data to every single node, when really it only needs to be part of the root

#

Oh, sorry, two types. ParentExecutorNode and ParameterExecutorNode. Similar names look confusing. Issue still remains though. Shouldn't need to pass in your plugin name into the parameter node

#

Then all passing the command name into ParentExecutorNode and your register() call. You've gotta find some way to deduplicate your parameter

sharp epoch
#
System.out.println("Total recipes: " + Streams.stream(Bukkit.recipeIterator())
                    .collect(Collectors.groupingBy(
                            recipe -> recipe.getClass().getSuperclass().getSimpleName(),
                            Collectors.reducing(0, e -> 1, Integer::sum)
                    ))
                    .entrySet()
                    .stream()
                    .peek(HowManyRecipes::printEntry)
                    .mapMultiToInt((a, b) -> b.accept(a.getValue()))
                    .sum());

what more clean™ ways to do this can you come up with

#

I know the simpler answer is just to use a while loop and count like a normal person, but I wanted to see if I could make it an one liner with a stream lol

#

I do wonder if one could avoid the double stream by using a stream gatherer but I have no idea how to use those

random yacht
#

Yeah, the grossest thing about that snippet is the Iterator -> Stream -> Map -> Stream pipeline you have going there. For this specifically, definitely just use the iterator. But seeing as you don't want to, yes, the only way to skip that intermediary Map step would be to use a gatherer, which is basically just a custom intermediary stream operation.

#

Bar that, the only improvement you could make is replacing mapMultiToInt() to just .mapToInt(Map.Entry::getValue). You're not mapping multiple values, so #mapToInt() works just fine

late smelt
#

Just uhh

#

toList the stream and .size

random yacht
#
  1. That would then require allocation of a List, which isn't necessary
  2. They also want to know how many of each type of recipe there is
#

i.e. ShapedRecipe vs ShapelessRecipe vs FurnaceRecipe, etc.

late smelt
#

Ah

late smelt
random yacht
#

For this specifically, definitely just use the iterator.
Yeah, I agree lol

sharp epoch
# random yacht Yeah, the grossest thing about that snippet is the `Iterator` -> `Stream` -> `Ma...

I did this instead:

logger().info("Total recipes: " + Streams.stream(Bukkit.recipeIterator())
                    .collect(Collectors.collectingAndThen(
                            Collectors.groupingBy(
                                    recipe -> recipe.getClass().getSuperclass().getSimpleName(),
                                    Collectors.reducing(0, e -> 1, Integer::sum)
                            ),
                            counts -> counts.entrySet()
                                  .stream()
                                  .peek(entry -> logger().info(entry.getKey() + ": " + entry.getValue()))
                                  .mapToInt(entry -> entry.getValue())
                                  .sum()
                    ))
            );
random yacht
#

I mean... that's definitely one way to do it

sharp epoch
#

it surprisingly doesn't matter at all which way you do it, it all ends up jitted the same way and runs in ~10ms

#

I tried a couple of different ways and it is all the same, at least for the recipe iterator

#

now that I think about it I could've merged that forEach with the stream below by using peek

#

I wonder whether a Multiset from Guava would be a bit more efficient in this scenario

random yacht
#

Yeah so you ended up with the same thing, only you've moved the chain into a Consumer as part of the Collector

random yacht
#

Map.Entry::getValue is valid btw instead of the lamda

#

Or just Entry:;getValue if you wanna import it separately, whatever

sharp epoch
#
Multiset<String> counts = HashMultiset.create();

var iterator = Bukkit.recipeIterator();
while (iterator.hasNext()) {
    var recipe = iterator.next();
    var key = recipe.getClass().getSuperclass().getSimpleName();
    counts.add(key);
}

counts.forEachEntry((type, count) -> logger().info(type + ": " + count));

logger().info("Total recipes: " + counts.size());

more traditional approach for those who were hurt by the one-liner

upbeat sinew
hasty lotus
jagged rock
hasty lotus
#

Well, I guess I could extend it and create my own factory class for it, but the whole point of cleaner is to ensure that if the user forgets to call close or release it doesn’t do bad things

random yacht
#

If the user forgets to call close or release, they're dumb 😂

sharp epoch
#

MediaPlayerFactoryProvider ☠️

#

some Spring MVC ahh code

hasty lotus
#

package me.brandonli.mcav.media.player.pipeline.filter.video.dither.algorithm.error

sharp epoch
#

even if the user forgets and stores a strong reference somewhere which avoids it being gc'd, they wouldn't create more than one factory at most so it filling up memory also wouldn't be an issue

#

I do agree that it should be an AutoCloseable if anything, as anything with a shutdown/release method is more conveniently handled with AutoCloseable as that permits using the resource with a try-with-resources, or .use in Kotlin

#

Cleaner is supposed to be used in scenarios where you can't afford to have the data lying around for security purposes, so even if someone forgets to call close then you can just clean it up from your end

#

in most cases though, it'll just become unreachable and eventually GC'd so it isn't particularly necessary for most scenarios

sullen hollow
novel meteor
#

I guess you're asking for feedback speccifically for the interface part, I can't do that, hopefully someone else will do.
I however have some different things.

Small personal opinion onShiftRightClick should not be called "Shift"
It's sneak. Shift is the default key and some people (like me) may have it on Ctrl

public Map<Integer, CustomItem> items = new HashMap<>(); the Integer being Custom Model Data
This is not ideal. What if I want to have diamond and iron sword both with CMD of 42. I can't, only the last registered one will be functional. I'd recommend storing item ID in custom PDC field.
Also, you might wanna look at the item_model component instead of CMD for custom models.

#

This is not how you use item meta. Each getItemMeta call returns a copy. That means you should create an ItemMeta variable, call getItemMeta once. Then do your checks from there.

#

Not a fan of you cancelling the event by default, should be handled by the item itself instead.
You can provide default cancel in your CustomItem interface. This way would be more flexible imo

#

And the very last thing

:(

dense leaf
#
    default void onRightClick(Player player, PlayerEvent event) {
        // Default implementation can be empty or provide some default behavior
    }

or provide some default behavior
screams AI generated

sullen hollow
#

Deleted them for the others, forgot that one ig

sullen hollow
sullen hollow
sullen hollow
# novel meteor I guess you're asking for feedback speccifically for the interface part, I can't...

Yeah, know that... Tho almost everyone still uses CustomModelData, i'd have to convert it for all my nexo packs myself, that is an additional step everytime I buy a pack. I will definetly take a look at Those new components as soon as texture pack makers start using them. Also I only use paper for custom items and if I want to create new tools I still only use model data I didnt use before. I got a pretty good structured list for that

sullen hollow
#

good call

novel meteor
sullen hollow
novel meteor
#

You're only using it for a simple model.
Just like a diamond sword, netherite hoe. That's it.
Custom Model Data now holds more than just one integer. That can be used for many things, custom colors for example. Maybe deciding what other models to composite to the current one.

sullen hollow
#

alr

#

Thx

#

Will try

novel meteor
#

Wiki has all the info about these features

dense pond
#
@EventHandler
    public void onInventoryClick(InventoryClickEvent event) {
        Inventory top = event.getInventory();
        Inventory clicked = event.getClickedInventory();

        // if blacksmith inventory is not open, return
        if (!blacksmithInventories.contains(top)) {
            return;
        }

        // player clicked on the blacksmith inventory
        if (clicked == top) {
            event.setCancelled(true);
            return;
        }


        // player clicked on bottom inventory
        ClickType clickType = event.getClick();

        switch (clickType) {
            case DOUBLE_CLICK:
            case SHIFT_LEFT:
            case SHIFT_RIGHT:
                event.setCancelled(true);
        }
    }
@EventHandler
    public void onInventoryDrag(InventoryDragEvent event) {
        if (!blacksmithInventories.contains(event.getInventory())) {
            return;
        }
        for (int rawSlot : event.getRawSlots()) {
            if (rawSlot < event.getInventory().getSize()) {
                event.setCancelled(true);
                return;
            }
        }
    }
#

trying to prevent player from editing the top inventory

#

didnt know how to check if double_click changes the top inventory, so i cancelled it no matter what

junior holly
#

If the point was to just cancel it entirely if it’s the top inventory, then it doesn’t really matter the click type. I noticed you didn’t include middle click so perhaps you’re trying to relay some functionality with that? As for the drag event, I’m not too sure what this trying to prevent but I feel like it’s not really necessary lol

late smelt
#

Certain click types will move items to the top inventory from the bottom inventory

#

Or vice versa

junior holly
#

I thought drag event was just anytime you selected the item so to speak

#

I guess I never looked into what makes it a “drag event” as opposed to a “click event”

late smelt
#

If you drag the mouse when placing items

#

Normally it’s for multiple slots but it can even trigger with a single slot iirc

dense pond
#

anyway i changed that function

finite juniper
#

?paste

rich fogBOT
finite juniper
#

rate

novel meteor
#

What is this supposed to achieve ?
Why do you need to use reflection to register the command

wintry fern
dense leaf
#

you're not cool unless you use unsafe

late smelt
#

Unsafe.putInt(0, 0)

#

I wonder why that breaks everything

#

What’s at index 0 in memory that doesn’t want to be set to 0

dense leaf
#

me

jagged rock
finite juniper
#

basically registering commands without needing to put it in the description file

pine grail
pine grail
#

perfect

round fossil
finite juniper
#

ok imma split the registration part

dense pond
dense leaf
#

that's a bit unecessary

dense pond
#

yep

#

and the code didnt look much shorter

novel meteor
#

Isn't reflection actually quite slow ?

dense pond
#

i dont think you get into situation when you have to register soo many lisdteners and commands

#

that you have to use reflections

dense leaf
novel meteor
#

And actually, doesn't the spigot event register have to use reflection ?

dense pond
#
private void registerEvents(Listener... listeners) {
        for (Listener listener : listeners) {
            getServer().getPluginManager().registerEvents(listener, this);
        }
    }

i use that thoguh

dense leaf
#

reflection isn't slow as long as you cache it

#

and definitely not slow enough to notice

wintry fern
#

however if used properly the trade off is worth it though

#

also as noted, you may not really notice the difference especially in things like plugins

jagged rock
#

I wouldn't use it for something critical

wintry fern
#

not sure what you consider critical

desert ore
sharp epoch
#

if you use cached MethodHandle/VarHandle(s) then it is pretty much the same as calling a method directly

round fossil
#

newer java versions are reimplemented to use method handles as well, the fast ones no?

#

like implementation wise

hasty lotus
#

The only way to code plugins faster is to like use libraries or just know what to do after coding so many plugins

#

Even if you were doing that approach where you register all listeners or something, I would use something like classgraph. I had to use it to register like 200 gadget implementations in my minigame

dense pond
#

Cause you do it once

#

But still

sharp epoch
#

the only way to make overall plugin development faster is to invest time into developing your tooling/configuring integrations

#

i.e. making a devcontainer setup, IDE templates, etc

hasty lotus
#

Using maven/gradle plugins to speed up your workflow so you don’t have to drag and drop a jar, etc

#

Developing a common library

sharp epoch
#

common libraries are more of a maintenance burden than they're help tbh

#

I stray away from those

hasty lotus
#

Me too I dislike them

sharp epoch
#

ig if it comes down to specific things then it should be fine, like database integrations and whatnot

#

but just a common library that is just a bunch of utiltiies is eh

desert ore
#

having a seperate library that just contains a bunch of utility methods could be pretty neat Ig

#

tell me one method that every plugin you made contains

hasty lotus
#

I guess but honestly it’s a burden because

  1. you have to basically maintain multiple projects at once
  2. you have to figure out if it’s ideal to be in a common library in the first place
  3. usually plugins are so specific and you don’t really need all of those utility things
#

Like do you need a database for a slime pad jump plugin? I think it’s just unnecessary code floating around

desert ore
#

but what about the lil stuffs that you always use

#

e.g. Util.t() to turn &'s into §'s

sharp epoch
#

I would use Adventure to begin with

hasty lotus
#

^

desert ore
#

very fair

#

very very fair

sharp epoch
#

but ultimately, those methods aren't worth a whole library

hasty lotus
#

Also Util.t() would be a nightmare

sharp epoch
#

like, the level of indirection is insane

desert ore
#

I personally always called it t because it was fast to type

sharp epoch
#

I just call it color or colorize and statically import the method

hasty lotus
#

Yeah

#

But I mean

#

Doesn’t happen in the first place

#

Use other libraries to handle it

sharp epoch
#

I mean, if I am just making a very small plugin, I can't be bothered to do adventure integration for the very few messages it may have

#

though ideally you'd have a plugin template with things like that and i18n setup so that you can distribute any plugin with configurable messages but eh

hasty lotus
#

Use a downstream fork 🏃‍➡️ /s

desert ore
#

I used to have a I18n thingy in my library but that shit was so ass

#

and the lib weighted more than the plugin anyway

sharp epoch
#

i18n is easy, people just like to overcomplicate it

sharp epoch
hasty lotus
#

I don’t use i18n actually

sharp epoch
#

it's something you want from the get-go since even if you don't have a lot of messages, as the plugin grows you'll want translatable messages anyway

hasty lotus
#

I usually just use adventure’s translators and then have my users just pick whatever language and it selects the properties file

sharp epoch
#

that isn't an adventure feature but a Paper one, no?

hasty lotus
#

No

#

It’s an adventure feature

sharp epoch