Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colored stairs and slabs V2 : now off by default! #1748

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

abel1502
Copy link
Contributor

I finally got around to implementing what I'd suggested in #1723 (comment). In short, now the stair and slab variants of colored and builder's choice concrete are locked behind an off-by-default config option. This way, no block IDs are sacrificed unless the users actually want it. I've tested this, as per usual, and it seems to work fine.

@HbmMods
Copy link
Owner

HbmMods commented Nov 2, 2024

image

@abel1502
Copy link
Contributor Author

abel1502 commented Nov 2, 2024

Quoting myself from #1723 (comment) :

Carpenter's block just feels too... frivolous? Having access to diagonal slopes and vertical slabs and whatnot detracts from the challenge of making minecraft structures for me.

I'd also say they way things are currently feels unnatural from the perspective of a regular user. Like, why is there a gray concrete slab but not a brown one, unless you install a completely separate mod? I fully understand your concerns about ids, but this pr feels like a perfect compromise to me

@Vaern
Copy link
Contributor

Vaern commented Nov 2, 2024

NTM is already on the backfoot when it comes to block IDs. We have exactly 0 need to use up 72 separate IDs for purely decorative blocks. On top of that, your method of adding blocks is exceedingly inefficient; stairs could cram in another block per ID (which is still weird and buggy, which is why we don't do this) but more importantly you use up two IDs for every single concrete color even though slabs can support 8 variations each.

@abel1502
Copy link
Contributor Author

abel1502 commented Nov 2, 2024

@Vaern

  1. Which is exactly why this PR does nothing unless the user explicitly expresses their wish to have these blocks by enabling a config flag (which is set to off by default)
  2. Please look closely at the code, I already do exactly what you suggest for slabs. Each slab block does indeed handle 8 variants simultaneously

@Vaern
Copy link
Contributor

Vaern commented Nov 2, 2024

Oh yeah, you're correct on that. For the stairs in particular, though, the amount of IDs used is still too many to be acceptable.

On the other hand, there are several reasons (which I'm sure Bob has already voiced) why we wouldn't want this even with a config. For starters, it would still affect our block ID budget because we have to account for the edge case where someone does actually decide to turn on the option; our options are effectively limited regardless, except the config now invites missing mapping errors if someone makes the mistake of touching the config (since it's registered on start and not save-dependent).

The config doesn't alleviate anything, we still may as well be consuming the IDs, and all for decorative blocks that can be easily replaced by carpenter's blocks. That mod feeling "too easy" is a non-issue, as you control the buttons you press and the blocks you place.

@abel1502
Copy link
Contributor Author

abel1502 commented Nov 2, 2024

@Vaern I'd argue you wouldn't have to account for the worst case, since users willing to opt for extra content may always install EndlessIDs. This is my personal plan for if I ever run out, considering I've already merged this into the build I'm currently playing on. Missing mappings are also only marginally an issue, since, fundamentally, if you disable a particular subset of blocks, they will have to disappear from the world one way or another, and clicking "yes" on a forge prompt is a fine way to do it.

@HbmMods
Copy link
Owner

HbmMods commented Nov 2, 2024

I don't think either the extra IDs or the mapping thing are issues (considering it's a config after all) but I'm still not fully convinced we need this, given Carpenter's Blocks already does this and more.

@Vaern
Copy link
Contributor

Vaern commented Nov 2, 2024

...

It's all complications that we could instead circumvent with another mod. That's my point.

@abel1502
Copy link
Contributor Author

abel1502 commented Nov 2, 2024

@HbmMods to me, this "more" is the problem with Carpenter's blocks. It's a big change to the game rules in terms of how decorations are supposed to be done. Stairs and slabs, on the other hand, are in line with vanilla Minecraft and the rest of the mod.

Also, perhaps a couple of screenshots of how I used those in my current base could sway your mind?😄
screenshot 1
screenshot 2

@Vaern
Copy link
Contributor

Vaern commented Nov 2, 2024

Once again, you can choose to just not use those blocks.

@abel1502
Copy link
Contributor Author

abel1502 commented Nov 2, 2024

@Vaern Sure, but then the same can be applied to this PR: you can just choose to not use any of its functionality.

Besides, the same logic could be applied to every stair and slab already in the mod. I think it is worth maintaining some consistency in this matter.

Also, restraining yourself from something you have trivial access to doesn't come for free. It's constrant temptation to cut a corner (literally, if we're talking about carpenter's blocks).

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.

3 participants