Skip to content

Conversation

@SuperKnux
Copy link

No description provided.

slava110 and others added 30 commits June 15, 2025 20:45
# Conflicts:
#	Fabric/build.gradle
#	Forge/build.gradle
#	Forge/src/generated/resources/.cache/d4b3e6634c30118e1127c02b727ea285752e5aac
#	Neoforge/src/main/java/at/petrak/hexcasting/forge/loot/ForgeHexCypherLootMod.java
* Attempted fix of Networking (Incomplete)
* Some minor fixes here and there
* Other fixes here and there (fixed up some whole minor folders so that's pretty cool)
* Fixed NBTUtils!
* New ContainerInput for Recipes

* Various edits to different Items

* etc, this is setting up for a merge anyway
…visualizing on impeti scrying sight mishap)

Fixed slate rendering both as a block entity and an item stack.
…n Fabric; might overhaul the registry with it.

* Registered ItemApiLookups (ParticleSprays function on Fabric!!!)
* Readded BrainsweepeeIngredient.getSomeKindOfReasonableIDForEmi(), EMI recipes work for the most part
* Fixed Fabric Creative Tabs (mostly)
…r other things.

* Fixed Fabric interop naming.
…(Not when erroring)

* Adjusted Fabric creative mode tab entries (untested)
* Fixed scrolls being unreadable
* Fixed rendering artifacts when both toasts and casting stack are present on screen
* Fixed Media Cube implementation killing server
* Adjusted neoforge.mods.toml
@SuperKnux SuperKnux marked this pull request as ready for review December 27, 2025 02:09
@object-Object
Copy link
Member

A note for future reviewers: you can hide all JSON file changes to make the page work significantly faster

image

@object-Object
Copy link
Member

Another note for future reviewers: if you use Bitwarden, apparently disabling its browser extension makes the review page work significantly faster???????

Copy link
Member

@object-Object object-Object left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly only reviewed Common, will come back and look through the platform code later. Left many questions/comments/suggestions.

I think my main concern is with how EntityIota was ported. I'm not sure how we want it to work, but I'm not satisfied with how it currently is. We can discuss it more on Discord.

Comment on lines +148 to +158
compileKotlin {
compilerOptions {
jvmTarget = JvmTarget.JVM_21
}
}
compileTestKotlin {
compilerOptions {
jvmTarget = JvmTarget.JVM_21
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already in subprojects, why do we need it here?

Comment on lines +9 to +12
pkSubproj {
platform = "common"
pkPublish = false
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary. We configure platform in the base build.gradle, and pkPublish defaults to false already.

Comment on lines +38 to +40
loom {
accessWidenerPath = file("src/main/resources/hexplat.accesswidener")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate.

@@ -9,6 +10,7 @@ architectury {

// FIXME: update dependencies/versions before releasing 0.12
pkSubproj {
platform = "fabric"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already set in the root build.gradle

modLocalRuntime "dev.architectury:architectury-fabric:$architecturyVersion"
modLocalRuntime "com.samsthenerd.inline:inline-fabric:$minecraftVersion-$inlineVersion"
modLocalRuntime "at.petra-k:paucal:$paucalVersion+$minecraftVersion-fabric"
localRuntime "hexcasting:jankson:1.2.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely we can get jankson from an actual maven? Why do we need to vendor it?

Copy link
Author

@SuperKnux SuperKnux Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the most reliable way via trial and error for me to prevent Fabric:runClient from failing due to a strange NoSuchMethodError related to Jankson; it's in the name 😭 (it randomly shows up or disappears, it's really weird)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but why do we need to include the jar locally? Jankson is on Maven Central. Also, can you put that context in a comment here?

GameProfile HEXCASTING = new GameProfile(UUID.fromString("8BE7E9DA-1667-11EE-BE56-0242AC120002"), "[HexCasting]");

boolean isBreakingAllowed(ServerLevel world, BlockPos pos, BlockState state, @Nullable Player player);

boolean isPlacingAllowed(ServerLevel world, BlockPos pos, ItemStack blockStack, @Nullable Player player);

<B> IXplatRegister<B> createRegistar(ResourceKey<Registry<B>> registryKey);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a doc comment

import java.util.function.Supplier;

/**
* Handles registration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a bit?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NeoForge registration uses deferred registry, fabric registration registers things directly, it's platform-independent interface useable in Common module

* Handles registration
* @param <B>
*/
public interface IXplatRegister<B> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the type parameter called B?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was like that when i got here 😭, i can change it to T instead

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's B because it is BASE class for registry entry.
MyItem extends Item, where MyItem is T generic parameter in register method of this class and B is base class of registry, type parameter of the class

Comment on lines +110 to +117
LootTableEvents.MODIFY.register { key, tableBuilder, source, lookup ->
if (Blocks.AMETHYST_CLUSTER.lootTable.equals(key)) {
tableBuilder.modifyPools { pool ->
pool.apply(SetItemCountFunction.setCount(UniformGenerator.between(1.0f, 2.0f)))
}
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Fabric version of changing the amethyst cluster's default drops for amethyst shards to half of its' original amount.
The 1.20 way of doing it no longer exists in 1.21, so I have to use FAPI to do it.

HexTags.Actions.PER_WORLD_PATTERN
)
) keyList.add(key)
keyList.sortWith(Comparator.comparing<ResourceKey<ActionRegistryEntry?>?, ResourceLocation?>(Function { obj: ResourceKey<ActionRegistryEntry?>? -> obj!!.location() }))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need !! here?

@github-project-automation github-project-automation bot moved this from 📋 Backlog to 🏗 In progress in Hex Casting Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

4 participants