ejektaflex / Kambrik

A Lightweight Kotlin-Fabric Library
https://kambrik.ejekta.io/
10 stars 4 forks source link

More Features and much needed buildsystem cleanup #9

Open isXander opened 2 years ago

isXander commented 2 years ago

build system cleanup and updating of libs

Entity Renderer AutoRegistrar

object MyRegistrar : KambrikAutoRegistrar {
    val EXAMPLE_ENTITY = "example_entity" forEntityType ... withRenderer ::ExampleEntityRenderer
}

this also prompted quite a few internal changes.

there are now capabilities for future auto registrations to have client or server environments rather than both

Item Group Builder DSL (kambrikx)

val EXAMPLE_ITEM_GROUP = kambrikItemGroup {
    icon { ItemStack(Item()) }

    items(
        ItemStack(Item()),
        ItemStack(Item()),
    )
    // or use actual list
    items.add(ItemStack(Item()))
    items += ItemStack(Item())
}.build(Identifier("example", "itemgroup"))

this is here for the future for when I can figure out how to elegantly implement this into KambrikAutoRegistrar but serves as a great standalone addition nonetheless

BufferBuilder DSL (kambrikx)

tesselate(1, VertexFormats.POSITION) {
    vertex {
        pos(1, 1.5, 1)
        tex(2, 2.5f)
        color(255f, 255f, 255f)
    }
    // etc...
}

This was suggested by me a long time ago but never got added, so here it is! In a PR!

ejektaflex commented 2 years ago

A few critiques:

Splitting mod registrar up makes sense - but I'm not sure if this is necessary? I have no idea why someone would register content on only the client or only the server, and I'm not sure if the niche use cases for this would justify making the API more complex, increasing the barrier of entry.

The ItemGroup DSL looks great! However, ~I would remove the icon function and the build function, putting both into function parameters of kambrikItemGroup~

Scratch that, now that I've typed that, I don't think a DSL/Builder is needed for it at all. If you do what I suggested above, then the only thing that you define inside of the builder is which items to add. This doesn't constitute the need for a DSL, you just need a list. When in doubt, KISS. I think that a method infix fun Idenfifier.forItemGroup(icon: ItemStack, items: List<ItemStack>) would be much more concise.

Lastly, I think that I am going to reject the buffer builder DSL that's been defined here - it does look really nice, but it does not fit all of the requirements of what I would consider needing a DSL. Most Java builders don't need to be morphed into a DSL, and vertex painting is very regular, ordered, repeated action. Additionally, I want to avoid doing an object allocation per vertex where possible (which might be the most pertinent real world concern)

Instead, I've preferred to simplify the process through extensions, such as: https://github.com/ejektaflex/Kambrik/blob/1.18/src/main/java/io/ejekta/kambrik/ext/render/ExtVertexConsumer.kt

More information on that decision can be derived from here: https://kambrik.ejekta.io/main/Philosophy.html#write-sensible-dsls And you can message me about it as well. Basically, if you can take Java code and wrap it in an apply with a .build() implicitly added at the end, then the benefits of turning a builder into a DSL are negligible.

However, I do think that an extension function for tessellate would be nice!

Version, dokka, and general build script updates all look good.