#[MEGA THREAD] Get Your Code Reviewed or Review Others
1 messages · Page 16 of 1
Idk if that'll be useful, but here is a BuildTools CI if anyone wanna use it, https://github.com/Paulem79/MultiNMS-Template/blob/main/.github/workflows/build.yml
Also, idk it it's related to this thread, but how can I optimize this CI, like, the BT generation is taking a lot of time
lot of idk lol
is there some tricks I can do to make it faster ?
it doesnt?
It won’t automatically be removed from the collection
But there are weak collections iirc which can handle that
oh u mean like the weak-references themselves are lazily removed?
Yeah
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
Can yall review this plugin? I made it and spent toooo much time for not being able to upload it in my aternos files.
https://www.spigotmc.org/resources/macecraft.123681/
I made the mace for 1.18.2
Aternos says I need popularity for this to be added to the plugins menu
where is source code
Don't tell me you mean star reviews 💀
Oh I need the source code? 😬🤥
we're not going to decompile it and give feedback on that
Nvm screw it. I was just asking for the actual review in like stars
Had a feeling that was the case
.
@tiny sleet do you give consent to decomp?
Sure
Im new to this stuff
Sooo don't expect anything good
ReplIt AI built this yeah?
Maybe..
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! 🎉
@tiny sleet
Well thanks for nothing 😒
💀
for nothing? Get a grip.
why are u making plugins again?
None of your business
damn I thought AI was better than this
messi>>>>
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
OH 💀
No not really. But I respect him though
???
The class is useless?
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) {}
I didnt make it..... remember????
So? This is a code review channel, you're getting graded.
Learn to code please :D
im very chalant
I really thought this server was help. I guess I couldn't have been more wrong 😒
Are you asking for a code review or a resource review
People just want to make Fun of my stuff.
just to be clear
Bruh
I never said I want u to give it 5 stars
Thats where it told me to go to when I said it in general stupid
Someone assumed you weren't begging for star reviews
and were asking for code feedback
Well that seems like a personal issue
I guess I'm better off without this stupid server's "help"
Nah I'm out
I guess that answers my question
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.
just because you use ConcurrentHashMap doesn't imply free atomicity
but also why use it there? in for example EnergyNetwork
My idea was that ConcurrentHashMap was needed because getComponentsConnections can be run async
there's quite a few things you could do to make it faster, but the main thing would probably be to cache buildtools output
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
sometimes its fine to not use a concurrent hash map even for async stuff, it really depends on how its used
In this cause do I both read and change things in it async
to my second point, you should expose ur getter of that map as the return type of a ConcurrentMap if concurrency is a requirement
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
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
Okay, what could I do to fix that?
one way is to use the factory method ofNullable, but maybe there's a better design u could implement around
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
that true
ah, you're checking against a components, but returning whatever componentConnections holds
Okay, but if I return ofNullable gonna it lie if you get what i mean?
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
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
Your read-write lock implementation in EnergyNetworkSerializer is completely flawed
is there a particular reason you're separating each EnergyComponent in their own collection? I feel like that complicates things for no reason
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)
Do you mean this?
yes
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);
}```
instead of filtering and findAny, you could just use noneMatch
components.stream().noneMatch(component -> component.getUUID().equals(uuid))
cool
tbh why no just return componentsConnections.computeIfAbsent(uuid,_ -> new ArrayList<>());
then u get free atomicity also
but wilmer this is important also
yea, I just want to finish the components connection code thing
fair, I should say if u do involve IO with async, please supply an executor to ur CompletableFuture instances
What do you man by atomicity?
atomicity means something happens as an indivisible action
other threads can either operate before or after that action, not in between
Emm, like this?
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
oh final word, it can be quite nice to take advantage of ReentrantLocks fairness policy when dealing with IO
ye sure i think thats good enough, I didnt study ur system to the letter but presumably :>
I personally don't understand that method to well either lol, I just picked it because the usage of filter and findAny annoyed meç
🥲
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
Its used to get connected components for a component in a network. If the component does not exisit in the network I will return empty, otherwise It should be an empty list that is in the map, or the value that exisit
Will computeIfAbsent add the list to the map, or do I need to do that myself?
it adds the list to the map
good
is it actually meant to not check the consumers and suppliers collections?
more reason to just unify these tbh
But do you belive its better to just have one list of components, and just get each type in the distributeRemainingEnergy & getRemainingEnergyConsumers methods?
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
computeIfAbsent normally does something like:
if containsKey return get
else put and get
but in the case of ConcurrentHashMap it does it a bit different (implementation wise), which is why it happens atomically ^^
then again, you'd call the filtering every time you update the network so that may as wel not be the best option
yea, that was my idea with diffrent lists
becouse the filter methods might gonna call every tick
and it can exist a lot of networks
yeah, I don't love it but it sounds sane enough
I do not love it either
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
yea like @implnote?
sure, that works, any documentation is better than no documentation
my favorite phrase, and ill say it again, code reads more times than it writes, so be mindful of what u write
that said, while it confused me for a hot second, after looking at it more carefully, the intent was clear so I think it is good enough, you can strive for elegance later if you feel like it but I personally like keeping my code simple if it can be helped
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
okay, how can I fix that?
you're better off having a single run task timer where you only call updateNetwork whenever a boolean is toggled via requestNetworkChange or whatever
oh, that runs every tick but might do nothing some ticks?
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
LMAO
is im stupid?
you just filtered them, you didn't cast them yet
List<EnergySupplier> componentsToUpdate = components.stream()
.filter(EnergySupplier.class:isInstance)
.map(EnergySupplier.class::cast)
.toList();
Thanks
Okay thanks, I'll try that !
I also was wondering why you were using a random clean artifact action instead of github's own one
I didn't find GitHub's one when searching on internet 😅
well, it is more so a setting on the upload artifact action
where should I use that?
Do you mean for the components list in EnergyNetwork?
Okay, in which way should I create it?
In which way?
myea
i mean if u pop and push a list a lot
its generally better to go with a linked list
pass an executor to it
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<>()));
}```
sure virtual threads work good enough
there are some problems with regional thread pinning for virtual threads in java 21
which is fixed in j24-25
i think, still not entirely sure what the code is doing but yea
So what would I better option be for jdk21?
Its used to get connected components for a component in a network. If the component does not exisit in the network I will return empty, otherwise It should be an empty list that is in the map, or the value that exisit
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
save json files with networks, every time a network changes something, maybe a player disconnect a component or a creeper blow the network up
I go with an newCachedThreadPool then
pinning shouldn't be an issue for you
I wouldn't worry about it, but if you care enough, just install java 24
okay
Should I have executorService just in the serializer class, or everywhere I use compleatublefuture?
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
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?
oh, that was my idea... What are im doing wrong?
ur program may remove a lock from the map while its being acquired by some thread
should I remove it before unlock?
do you mean
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
okay, so how could I fix it then?
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
UUID isnt for the player, its the networks uuid
it was an example
oh
so I need an expiration time then?
or might just one lock for everyhing be better?
no then u'll spend more time waiting for the lock than ur program doing useful stuff
okay, but I do not known how to fix it...
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
Okay, but how should I known if I should expire the lock?
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 ^^
how do I do that?
okay, but should I just remove it after 10 minutes. But what will then happen, if it is used serveral times, so it might be used when I will remove it after 10 minutes
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
okay, but could it happen that I reset the expiration the same time as it removes it?
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
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
I not get how to use it compelty do I need my conurrentHashmap with locks?
its a replacement to concurrent hash map
LoadingCache<K, ReadWriteLock> = Caffeine.newBuilder()
.expireAfterAccess(Duration.ofMinutes(10))
.scheduler(scheduler)
.build(_ -> new ReentrantReadWriteLock(true));
for example ^^
Where should I do that code, every time I want an lock, and do I need to remove it or something in some way?
what I gave u is basically
Map<K,ReadWriteLock> = new ConcurrentHashMap<>();
but it has built-in expiration and load-if-absent policies
Do I need to save the LoadingCache or something more then doing like this?
was that a "no", or "you are doing everything wrong"?
Like this then?
https://paste.md-5.net/edageyefum.cs
ye sure
was it anything more that might be good to change?
You said that you wanted better and more java docs right?
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
yea, I have tried comment the most complex part of the code, but I known some places that might need some docs
Oh okay thanks I see
https://github.com/SpraxDev/Action-SpigotMC/ imagine if this exists too
imagine
(thanks)
but with this, it's at least for a day, I just don't want them to be available
Contribute to orang3i/Gangs development by creating an account on GitHub.
yo i need feedback on my db impl
thanks fixed it 💀
i just learned without it sql injections are ez
far from finished but just gonna throw this here https://github.com/radstevee/axi/tree/master/game
🙏
So unholy people don't want to touch it or something Idk
i know the reason (kotlin)
Uhhhhhhh bad
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?
I do think he wants to get it reviewed
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>, )
It means the context is a constructor parameter
because sometimes I want the actual generic types
and also wanna be consistent
if I use it in some places and don't in others, it's a bit annoying
well but I mean like why does the context need a generic of the instance
you know its gonna be a game instance anyways
because it needs the GameType<T> which has a game instance factory of T
yeah but like you made both of those things
hm?
well its like my argument now shifts to why does GameType have a generic (presumably) game instance
I think what steaf is really arguing about here is whether you actually need a system wide type construction
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
it's not a kotlin thing, in my case just a preference thing I would say
and then I wonder why you did it that way
I like not having to cast return types
see here
yeah but in that case the method could just have a generic
instead of the whole class
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?
what does the def of BedwarsInstance look like
then I'd have to cast the factory which is mehh
yeah I guess its fine, its just bedwarsinstance plastered all over the place
fairs
but idk how you would possible change that
I don't think there's any good way to without doing a billion unchecked casts
I guess I would just use more of a compositional style compared to inheritance and generics
what does open mean in kotlin
means something can be extended (classes) or overriden (members)
everything is final by default
correct
kinda like c++ then ig
I prefer that because people can't just randomly extend all your shit without it being intended
but with the actual enforcement probably
yeah on c++ you'd enter the shadowrealm
there's a compiler plugin that lets you place an annotation on a class or a file to make everything open
yeah but that feels like its missing the point
yeah it's mostly for stuff like spring
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
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)
what does reified mean
means you get the class of a generic
wdym exactly
java has type erasure
you can get the class of T, usually you have to pass it in
like if you make a waitingphase, wouldnt it just be BedwarsWaitingPhase<BedwarsGameInstance>
example:
inline fun <reified T> printReified() {
println(T::class.simpleName)
}
printReified<String>() // prints 'String'
fun <T> print() {
println(T::class.simpleName) // compiler error!
}
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
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
I'm not sure I fully understand what you're trying to say, but if you pass in a BedwarsWaitingPhase (extends GamePhase<BedwarsInstance>), the class will be KClass<BedwarsWaitingPhase>
elaborate?
well im just trying to find out why you have those 3 classes separate
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
it feels like your design is suffering unnecessary type constructions just because u wanna feed in a type aware context in the beginning
do you mean I should use less generics?
also ur error handling can be improved
it kinda explodes to the whole codebase
less generics is the wrong emphasize
yea I wasn't sure what exactly it should do tbh
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
the question is just - how do I determine whether an error is fatal?
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 ^^
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
that's fair
I'm probs gonna start by removing it from the world stuff since that's quite unnecessary and never even used
was that related to this? #1100941063058894868 message
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
Kotlin has Result<T> with runCatching, but that's ultimately just a Throwable and not any kind of typed exception
I also hate when there's only getter methods that throw if the element doesn't exist and don't return null or any optional, it's annoying to have to wrap it in a try/catch
well kotlin has runCatching { ... }.getOrNull() for that stuff, it's still just annoying
fair enough
yes and no
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 ^^
usually it comes down to whether the exception is checked or not
if it is checked, it is a recoverable exception
rather, kotlin doesn't have checked exceptions, so yeah, a bit of an issue
you could do that as long as your users properly wrap their exceptions into runtime exceptions or whatever super exception you may impose
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
I’d disagree here
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
That I can agree with, mostly because in the plug-in development space people don’t know how to use exceptions at all
oh yea, most definitely true
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
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.
Some exceptions like ISEs, IAEs, NPEs and UOEs maybe?
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
coroutines work similar to goroutines right?
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
Yeah coroutines don't do that
but you can add coroutine exception handlers to the context
and if it doesn't apply to your "filter" of some sort, you can just re-throw it
Okay tbf when you do CoroutineScope.async you get a Deferred<T> back, which is similar to a CF
but that's also just a Job which can have completion callbacks and such
(You can also just convert them to/from CFs)
See https://github.com/radstevee/axi/blob/master/core/src/main/kotlin/net/radsteve/axi/coroutines/ExceptionHandlers.kt and then you can just do ```kt
public val AxiPlugin.syncContext: CoroutineContext get() = minecraftDispatcher + LoggingExceptionHandler
CompletableFuture or 6mb jar bundle
Icl tho, CF has so many dependencies as well, its not like its just one class, tho ofc jdk is always there so
Library loader be like
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
Yea, you can also advocate for implementers and users to use some custom exception where u pass that as cause ^^
entirely up to u
Like if any of the listed exceptions are thrown it throws a custom exception with a backing reason of whichever the original was sorta thing?
I have a GameInstanceException already with a subException function I think that'll do
but yea I do wanna mention rad, sometimes it might be the case that NPE is unrecoverable, i think its just so contextual
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
what is state here
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
wsg nerds
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)
coding go brt
https://github.com/radstevee/axi/tree/master/example%2Fsrc%2Fmain%2Fkotlin%2Fnet%2Fradsteve%2Faxi%2Fexample%2Fgame demo game for my game engine, can I have some feedback pwease
computer says no
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
I'll support mini but I really hate it tbh
string regex replacer >>> adventure replacer
Better than Kotlin DSL
I like my text builder
Olivo is a real kotlin hater
idk why but adventure replacers are so slow
I mean could just like.. register(::TntTagIntsance, GameTypeMetadata(...))
yeah literally serializing them back to a string and replacing them is about 20x faster
deadass
Make it faster
Damn
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
You can just String.mm and ComponentLike.mm everywhere ig
or do I wanna do .mini, might be clearer
minimize string?
no
meh
I think paper uses the term rich right to refer to minimessage parsing?
I personally really hate MM inside source code
if you are going to use it, provide a language file instead
I do agree whatever that buildText is ends up being ugly though
https://github.com/radstevee/axi/blob/master/ui/src/main/kotlin/net/radsteve/axi/ui/text/TextBuilder.kt yeah I don't like this at all, looks like a pointless abstraction to me
Kotlin Coroutine-based Minecraft game engine. Contribute to radstevee/axi development by creating an account on GitHub.
buildText {
append("TNT Tag")
red()
}
vs just
text("TNT Tag", RED)
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
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
Yeah lmfao
cool, i think api for cancellation might be nice
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)
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!!!
Conclure once again saying words that make my brain confused
PECS
^^
With fancy words
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
}
maybe use bytebuddy to generate async versions? idk
Yes
private final ListMultimap<Class<? extends MurderRunEvent>, EventSubscription<? super MurderRunEvent>> subscriptions;
obv java is still limited by use-site variance
yea
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)
Actaully
hii Pulse
Could I use just ? instead of ? super MurderRunEvent
yeah
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
is just ArenaEvent or LobbyEvent callable on its own?
so if I were to subscribe to LobbyEvent that'd yield an error, same to MurderRunEvent I assume?
Yes
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
but extending this, it can be nice to be able to listen to generic events sometimes, like lets say u wna log whether the game enables, disables or switches phase etc
so maybe an annot to decide if an event is invokable?
Yeah
i might as well port this to its own lib
Yeah I use checker
checker gets in the way sometimes...
so when i get lazy
i just add a @SupressWarnings("all") // checker 💀
and i deal with it later
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
I'm going to use a Stack to recursively go through parents for a bfs
Yea i origianlyl wrote it into recursive method but i didnt just cuz
yea no java is big troll when it comes to recursion
It's not that often you hit a stackoverflow when doing recursion and it being a stack limit rather than a bug
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
Anyways I'm off to get some sleep, have a good night :)
sleep well ^^
@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
looks nicee ^^
now i have to make like
100 empty interfaces
and then add event calling
code
💀
i just got have a lot of events to make
Actually
I could probably conjoin many events together
with some State enum or something
you see I have a colour registry system which allows you to define theme colours in a centralised way and they apply to all the default colours as well, which is not possible with regular component builders
why not
you could just make your color registry extend RGBLike and go ballons
because then you need to actually specify the color registry entry instead of TextBuilder's red() which just looks it up from the registry
do you have any example usage of your color registry? I need to see its utility before judging
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
I don't understand why ColorRegistry needs to exist for this, are these colors used for anything else besides chat messages?
no, but they allow you to define the colours once and then use them everywhere
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
I do that in a different project and just find it annoying tbh
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
true I could make a theme system
ew minimessage
newTagger.send {
append("You are now the ")
append("tagger", red, bold)
append("!")
yellow()
}
better?
lolol
definitely better, though I am still not too sure of the point of this abstraction over adventure lol
I didn't think you'd take what I said and actually make a theme system lmao
because I need some extra utility stuff and it's a horrible pain to import extension functions with the same name of regular members
kotlin problems kek
but yeah I did just make a theme system
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
good idea yea
@round fossil i remember u were talking about FSM earlier
yea
I think my minigame is sorta an FSM? I have a start, end
And sort of update
And then status
so u have states
Yeah
i just dont formally defined them as that
Should I formally turn them into like defined states?
i mean u have like
and use some library to assist
transitions also
ugh tbf I just end up designing it myself w a couple of sealed interfaces
not worth making any library for it
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 ^^
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,
}
}
with events u'll usually have one major concerned set of states, that takes u from start state to finish state
might not be atomic tho
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)
don't use atomic wrappers for no reason, they're expensive
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
reminds me when people started using AtomicInteger as counters inside runnables because it has that getAndAdd method
yea, I mean tbf I think AtomicInteger now has plain methods
like sure, that works but definitely not what you should be using it for lol
with no memory effects
I checked and there are more methods regarding all specific types of memory effects lol
yea
now you can choose between acquire/relase, opaque and volatile
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
plain too, I just had missed it
haha yea new stuff
like for example memory fences are still only through varhandle api by static calls
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
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
just let me manage memory with Unsafe#allocateMemory for maximum performance™
:,)
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
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
Have you ever had to dig for sun.misc.Unsafe to get access to CompareAndSwap? Are you interested in eking out the last bit of parallelism from your code? VarHandles in Java 9 enable memory ordering and atomic operations on any field type. With more granularity in the memory ordering modes, Java programmers can improve the potential parallelism o...
oh yea but that video doesnt go too indepth about it
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
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"
I still find it to be a pretty good introduction to these APIs, the rest you hopefully just understand while playing with lower level languages as you mention
yea
I was saying the exact same thing you just said but in different words lmao
but yea I think the Javadocs are maybe just very poorly written
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
if anything, they don't go nowhere near as depth as they did in others like the Foreign Memory and Function API
cuz like weakCompareAndSet fails spuriously isnt something one understands until u've done concurrent and multithreaded programming in lower levels
yea
oh yea
I hope they maintain the same level of care for the new vector APIs, I wanna play around with that
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
yea
I mean Ido get that there are a ton of issues to resolve while keeping backwards compat
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
for example union types tend to complicate type inference, adding null-restricted types must be done through an additive way etc
but it doesn't look like to be the case
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
well most use j21
in where some major issues were still present
or should I say are present
meanwhile me having to manually compile jetbrains runtime 24 😔
wish they'd just release binaries for non-lts
they even have a 25 branch already, just not much done yet
502: Stable Values (Preview)
503: Remove the 32-bit x86 Port
not too bad
structured concurrency api 🙏 pls no more previews
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
looks cool
ye huge
an L for 32 bit enjoyers tho- has to happen sooner or later tho
powerpc and arm32 ports were also removed not too long ago
bad time for 32bit enjoyers
literal("TNT Tag").color(RED) is how I do it lol
What about using the java.lang.reflect.Proxy class?
You wouldn’t have support for base classes or any inheritance
You would have to manually do that for Proxy
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
Did someone delete the messages
me
What did I miss :(
I saw someone do one class per type but i was wondering if there was an easier way
what
per type as in?
You started a question after skipping the prerequisite information lol
one class per type... i mean, yeah? that's a type
I'm guessing they mean an interface or abstract class with all the methods needed and an impl for each db type
doubt
this entire package contains some really weird code
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..
what does exactly that mean?
i want to learn but i really dont know if theres a better way
if you can give an example please 🙏
ill learn about that later, thank you
Use object xyz rather than a class with a companion object
You use it in some other places iirc
oh objects are for top-level singleton right?
objects are singletons
Does anyone have a proper registry impl that I could take a look at
Minecraft’s is kinda nice
Tho that is if u wna have a “freezable” design
Like this is my current registry https://github.com/PulseBeat02/MurderRun/blob/main/src/main/java/me/brandonli/murderrun/game/gadget/GadgetRegistry.java
But as you can tell, it's pretty simple and ass lmao
https://github.com/NormalManV2/NormalAPI/tree/master/src/main/java/org/normal/api/registry
https://github.com/NormalManV2/NormalAPI/tree/master/src/main/java/org/normal/common/registry
I was called upon, however this isn’t like the best impl so do as you will kek @hasty lotus
@SuppressWarnings("all") // checker
@hasty lotus i so feel u
just so you're aware, you're not allowed to sublicense under non-GPL compatible licenses when using GPL libraries
Thanks ill take a look
Yeah lmao i cba
is MIT not compatible with GPLv3?
you need GPLv3
MIT is compatible with gpl
but this is due to the fact it is the least restrictive
it was late
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
this is false
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
I am not so sure that's the case
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
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
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.
i assume u didnt make the pipeline monadic
Yeah I used like a builder pattern
is that bad
tbh its really hard to tell how u designed this
since we just see some method calls and some variables
hold on let me upload code
https://paste.md-5.net/aromuyifek.java
I feel like the physics are pretty much there, somewhat simple impl... fixed my throw mechanic so now you can actually decide on a specific power (used for the initial disc velocity calcs).
eGirlCatie
that's his alt
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?
I believe in one or two places
Modifying it during a task at least
Probably would be fine without though to be fair
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
Duplicated where? It's just the base of the flight physics
All the other physics methods are basically just modifications to applyPhysics
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);
}
yea that's nicer
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
Yeah I have yet to do that, but a lot of those values are still being tested
other than that, I don't really have much to say about the code, the structure looks good so far
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
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
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
Sureeeee… “alt”
wait till you find out my alt is called femboyyy_ now
I feel like I already knew that
this is great improvement
The code or the looks kek
the looks, don't care about the code as long as it works lol
Sorry I got it
This is the concept
Internal generation
https://paste.md-5.net/ehituxojew.java
Chunk/World gen integration
https://paste.md-5.net/sevodesuzo.java
Post wrold generation : Obstacle generation
https://paste.md-5.net/ametonuxum.java
I feel like the obstacle offsets need to be randomized a bit more, it still gens in rather straight lines
I also figure I can integrate the obstacle gen with the chunk generator, just haven't looked into that enough yet
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"
}
What's the reason for passing a string for the plugin name when you have a plugin instance right there?
So you can do formatting on the plugin name used for the text in the brackets
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
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
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
- That would then require allocation of a List, which isn't necessary
- They also want to know how many of each type of recipe there is
i.e. ShapedRecipe vs ShapelessRecipe vs FurnaceRecipe, etc.
Ah
One could argue a stream in general is unnecessary
For this specifically, definitely just use the iterator.
Yeah, I agree lol
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()
))
);
I mean... that's definitely one way to do it
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
Yeah so you ended up with the same thing, only you've moved the chain into a Consumer as part of the Collector
more or less, yeah
Map.Entry::getValue is valid btw instead of the lamda
Or just Entry:;getValue if you wanna import it separately, whatever
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
Actually I’ll probably rename the node names to be less confusing
I'm not entirely sure if I used the Cleaner class correctly. I have a singleton which has access to Native resources, and I'm using the Cleaner class as a safety net if the user doesn't properly call the release function. Would something like this be the proper usage? https://github.com/PulseBeat02/mcav/blob/master/mcav-common/src/main/java/me/brandonli/mcav/media/player/combined/vlc/MediaPlayerFactoryProvider.java
I'd just encourage try-with-resources ngl
How would that work if I have a static constant?
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
If the user forgets to call close or release, they're dumb 😂
package me.brandonli.mcav.media.player.pipeline.filter.video.dither.algorithm.error
the factory doesn't seem to contain sensitive data, so I don't really see the point of the cleaner tbh
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
First time really working with interfaces and stuff... Could anyone take a quick peek at my src code and tell me what I could do better or if I miss some important thing?
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
:(
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
Yeah, copilot suggested those and I liked them
Deleted them for the others, forgot that one ig
Yeah... Wanted to test papers plugin thingy for so long now, cant tell a difference yet tho...
Ill do, I was unsure if I should cancel or not, thought would be easier bc most items are going to be canceled
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
Will rename to sneak click
good call
Custom model data still has its uses in resourcepacks. But not in the way you're using it.
custom model should always be used in this case regardless of "everyone else"
What do you mean in "my case"?
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.
Wiki has all the info about these features
@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
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
Certain click types will move items to the top inventory from the bottom inventory
Or vice versa
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”
If you drag the mouse when placing items
Normally it’s for multiple slots but it can even trigger with a single slot iirc
yes
yeah and i cancelled it
anyway i changed that function
?paste
What is this supposed to achieve ?
Why do you need to use reflection to register the command
because you are not cool unless you use reflection
you're not cool unless you use unsafe
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
me
having to use reflection to even get it is such a nice test of knowledge
that's a part of a lib
basically registering commands without needing to put it in the description file
👍
?whereami
perfect
I’m not against the functionality at hand, but the quality is somewhat poor, isolating huge chunks of logic in a constructor tends to do more harm than good and often kills maintainability and flexibility
;p;
makes sense
ok imma split the registration part
yesterday i watched a video titled "2 tips to code plugins faster" and he said to use reflections to regiter all listeners in a package. i was like what? what about different constructors etc, weird
that's a bit unecessary
Isn't reflection actually quite slow ?
i dont think you get into situation when you have to register soo many lisdteners and commands
that you have to use reflections
not that noticably
And actually, doesn't the spigot event register have to use reflection ?
private void registerEvents(Listener... listeners) {
for (Listener listener : listeners) {
getServer().getPluginManager().registerEvents(listener, this);
}
}
i use that thoguh
yeah
reflection isn't slow as long as you cache it
and definitely not slow enough to notice
it depends, in general yes
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
I wouldn't use it for something critical
not sure what you consider critical
Isn't reflection cacheable most of the time
yes
if you use cached MethodHandle/VarHandle(s) then it is pretty much the same as calling a method directly
newer java versions are reimplemented to use method handles as well, the fast ones no?
like implementation wise
How tf is that related to code plugins faster though lol
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
Idk
Cause you do it once
But still
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
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
common libraries are more of a maintenance burden than they're help tbh
I stray away from those
Me too I dislike them
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
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
I guess but honestly it’s a burden because
- you have to basically maintain multiple projects at once
- you have to figure out if it’s ideal to be in a common library in the first place
- 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
I would use Adventure to begin with
^
but ultimately, those methods aren't worth a whole library
Also Util.t() would be a nightmare
like, the level of indirection is insane
I personally always called it t because it was fast to type
I just call it color or colorize and statically import the method
Yeah
But I mean
Doesn’t happen in the first place
Use other libraries to handle it
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
Use a downstream fork 🏃➡️ /s
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
i18n is easy, people just like to overcomplicate it
Even then I'd have to setup i18n which is just annoying
I don’t use i18n actually
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
I usually just use adventure’s translators and then have my users just pick whatever language and it selects the properties file
that isn't an adventure feature but a Paper one, no?