Fabulously-Optimized / fabulously-optimized

A simple Minecraft modpack focusing on performance and graphics enhancements.
https://download.fo
BSD 3-Clause "New" or "Revised" License
872 stars 84 forks source link

FabricBetterGrass #845

Open UltimatChamp opened 4 weeks ago

UltimatChamp commented 4 weeks ago

CurseForge link

https://curseforge.com/minecraft/mc-mods/fabricbettergrass/

CurseForge Mod Distribution

Allowed

Modrinth link

https://modrinth.com/mod/fabricbettergrass

Source/other link

https://github.com/UltimatChamp/FabricBetterGrass

Mod file size

130.91 KiB

License

Apache License 2.0

What it does

OptiFine's Fancy and Fast better grass on Fabric!

Why should it be in the modpack

Additional details

TheBossMagnus commented 4 weeks ago

Testing the mod kind of works. I have these weird zones where the effect isn't applied in fancy mod. 2024-05-28_13 10 10 Also switching form off/fast/fancy requires a quit and rejoin of the world, and this is not stated anywhere.

A couple of other things:

Madis0 commented 4 weeks ago
UltimatChamp commented 4 weeks ago

I have these weird zones where the effect isn't applied in fancy mod.

That's how it's supposed to work, isn't it? The places where the grass block isn't connected with another grass block below will not be better-grassified.

Also switching form off/fast/fancy requires a quit and rejoin of the world, and this is not stated anywhere.

Hmm... I'll place a comment then, warning the user.

Have you had any contact/authorization of the original mod dev?

No 😕, but the author has abandoned the project, so, I'll doubt he will have any objection. Moreover, it looks like he's not active on GitHub, nowadays...

it's possible to add a toggle for the mod in the video settings

great idea! i think i'll put that in sodium's Quality settings, in the next release

had a concern about the current Continuity implementation not being sustainable. Is that addressed in this fork?

currently, the mod is using the same implementation. I'll take a peek at that...

Madis0 commented 4 weeks ago

No 😕, but the author has abandoned the project, so, I'll doubt he will have any objection. Moreover, it looks like he's not active on GitHub, nowadays...

The original mod is made for a specific server and modpack, hence it is made for a specific Minecraft version and not updated very often. You should contact said modpack's owner/devs (see their Discord link on Modrinth) to actually know about it.

TheBossMagnus commented 4 weeks ago

That's how it's supposed to work, isn't it? The places where the grass block isn't connected with another grass block below will not be better-grassified.

I remembered that it blended with the block below and on the side (like with a dirt side if the block below it's dirt, stone if it is stone... But maybe it was another mod/resourcepack

Hmm... I'll place a comment then, warning the user.

Isn't possible to force-reload the render (like sodium does i believe)?

UltimatChamp commented 4 weeks ago

image

UltimatChamp commented 3 weeks ago

I am also curious about any talks with the original dev, maybe they would be willing to upstream the work you've done? Or better yet, maybe you two can collaborate to maintain the original mod together?

I've reached the author privately on Discord, and he seems to be cool about me maintaining the fork! I'm also going to split the revenue with the author, once he accepts the revenue request on Modrinth (done!), which he has agreed to (he didn't want the revenue actually).

UltimatChamp commented 3 weeks ago

I need some opinions: Should I JiJ MidnightLib like Puzzle has?

Madis0 commented 3 weeks ago

I need some opinions: Should I JiJ MidnightLib like Puzzle has?

I would prefer if you didn't, as it reduces redundancy. However, as you mentioned - it is really small, so ultimately it doesn't matter; you can include it if you feel it improves the UX for users.

UltimatChamp commented 3 weeks ago

had a concern about the current Continuity implementation not being sustainable. Is that addressed in this fork?

Continuity's SpriteCalculator class isn't really much of a work to port in the mod (merely a copy-paste)! If PepperCode1 decided to remove the class from the mod in the future, then the only effort I'll have to do is taking permission from them to use it in the mod.

So, I'll continue to include Continuity as a dependency until that actually happens.

Madis0 commented 3 weeks ago

had a concern about the current Continuity implementation not being sustainable. Is that addressed in this fork?

Continuity's SpriteCalculator class isn't really much of a work to port in the mod (merely a copy-paste)! If PepperCode1 decided to remove the class from the mod in the future, then the only effort I'll have to do is taking permission from them to use it in the mod.

So, I'll continue to include Continuity as a dependency until that actually happens.

Hmm. Well, the older version of that class is already available under LGPL, and according to your code, you only refer one method from it anyway.

So I think it would reduce a lot of complexity and fragility to just copy that one method over (or the code parts you need from it) and already not depend on Continuity.

UltimatChamp commented 3 weeks ago

Hmm... I've now dm'd Pepper regarding this.

UltimatChamp commented 3 weeks ago

I've just ported the mod to 1.21 (there were some changes to Minecraft's ModelLoader class)