Skip to content

mehvahdjukaar's refactors#65

Draft
quat1024 wants to merge 3 commits intomainfrom
refactor-mess-test
Draft

mehvahdjukaar's refactors#65
quat1024 wants to merge 3 commits intomainfrom
refactor-mess-test

Conversation

@quat1024
Copy link
Collaborator

Just opening so i can look on github

Copy link
Collaborator Author

@quat1024 quat1024 left a comment

Choose a reason for hiding this comment

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

Neat

ResourceLocation name = internalNames.get(entry);
z.log.debug("Registering to {} - {}", registryRes, name);
event.register(keyGeneric, e-> e.register(name, entry));
//TODO: nuke
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This exists so things like the slab block can set their ID to a variant of the full block's ID

return 0;

//TODO 1.20: weird
//TODO: wtf is this??
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Jank to set block flammability based on the "material" after 1.20 deleted all the materials. Plugs into IForgeBlock#getFlammability

Copy link
Contributor

Choose a reason for hiding this comment

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

ah so its an automatic flammability guesser

Comment on lines +33 to +37
Zeta zeta = module.zeta();
zeta.renderLayerRegistry.put(this, RenderLayerRegistry.Layer.CUTOUT);
zeta.registry.registerBlock(this, regname, true);
zeta.creativeTabs.addToCreativeTab(CreativeModeTabs.BUILDING_BLOCKS, this);
zeta.creativeTabs.addToCreativeTab(CreativeModeTabs.REDSTONE_BLOCKS, this);
Copy link
Collaborator Author

@quat1024 quat1024 Mar 29, 2025

Choose a reason for hiding this comment

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

This standardization is a much-needed change 👍

I wonder if they should be located from getters on ZRegister instead of grabbing them from Zeta... that way you could just pass ZRegister into the block constructor itself, and then I don't think these registries would have to be public on Zeta. Would require a million changes in quark though so this is a good compromise

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I thought about this. Making those changes would mean going into a direction where it would make sense to keep all registration stuff private and only acessible via ZRegister event (just like it usually is on forge). That could avoid some mistakes and be cleaner API but at the cost of needing bigger refactors.
In other words if that was to happen the access to the Zeta object would be relegated to read-only information and the actual registraction part would always have to be done via the ZRegister event


import java.util.function.Supplier;

//TODO: merge into ZetaSide
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wait lol what's this class doing here in the first place

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like it's just used to create Quark's sided proxy lol?? Will nuke the one (1) usage quark-side and then you can nuke this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

VazkiiMods/Quark@43450a7 alright yeet it

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Sure feels nice removing stuff

return b;
default Block getBlock() {
return (Block) this;
}
Copy link
Collaborator Author

@quat1024 quat1024 Mar 29, 2025

Choose a reason for hiding this comment

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

(meant to comment on the removal of setCreativeTab but github fucked up)

Deleting this will require a lot of changes in Quark, the method was put in this interface as a "builder" (ex https://github.com/VazkiiMods/Quark/blob/772bdfac2cd290d3c1d3c66a79e3790e99de1271/src/main/java/org/violetmoon/quark/addons/oddities/module/BackpackModule.java#L109 )

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah i see. Reason I didnt like it and didnt like the condition one is because of access. In theory any mod could get a zeta block and assing a new tab and such.
Obviously not much of an issue if only quark uses zeta but some of these changes were made to provide a cleaner API, in a way that would make the mod a solid choices for other possible mods that might want to develop a module driven mod.

One possible solution i could think of would be to make zBlocks accept a tab and condition object or maybe a general ZProperties object that takes care of the configuration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A more advanced block properties would be really great

Copy link
Contributor

Choose a reason for hiding this comment

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

so could be a new prop object or a vanilla prop wrapper. imo a dedicated one would be clearer and less invasive

Copy link
Contributor

Choose a reason for hiding this comment

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

tho tbh not quite sure as adding a prop object to blocks constructor wouldnt be something that the interface could enforce...

Copy link
Contributor

Choose a reason for hiding this comment

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

still given all of those are created in the zeta event it wuld be just one extra line before it that manualy specifics the tab... i dont think that would make it any less convenient really

Copy link
Contributor

Choose a reason for hiding this comment

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

to add to this, this would actually be a good argument for giving ZRegister to the QuarkBlock class. now IT could indedd do the automatic stuff like adding to the tab and what not when also given a tab in its constructor

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this diff just imports/whitespace/formatting stuff

Comment on lines +181 to +182
//whats this for?
public void addOnReloadListener(String id, Consumer<IZetaConfigInternals> consumer) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was for the "hide removed items from JEI" option? Safe to delete, no usages in Quark atm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants