search

FabricMC / yarn

Libre Minecraft mappings, free to use for everyone. No exceptions.
Creative Commons Zero v1.0 Universal
915 stars 377 forks source link

Block suffix inconsistency for block sound group names #2470

Open haykam821 opened 3 years ago

haykam821 commented 3 years ago

Some fields in the BlockSoundGroup class are suffixed with 'block'. Their sound events may or not have the suffix as well. Should this suffix be dropped entirely, or added to all necessary fields?

The following fields are affected:

Field Name Break Sound Step Sound Place Sound Hit Sound Fall Sound
SLIME BLOCK_SLIME_BLOCK_BREAK BLOCK_SLIME_BLOCK_STEP BLOCK_SLIME_BLOCK_PLACE BLOCK_SLIME_BLOCK_HIT BLOCK_SLIME_BLOCK_FALL
HONEY BLOCK_HONEY_BLOCK_BREAK BLOCK_HONEY_BLOCK_STEP BLOCK_HONEY_BLOCK_PLACE BLOCK_HONEY_BLOCK_HIT BLOCK_HONEY_BLOCK_FALL
CORAL BLOCK_CORAL_BLOCK_BREAK BLOCK_CORAL_BLOCK_STEP BLOCK_CORAL_BLOCK_PLACE BLOCK_CORAL_BLOCK_HIT BLOCK_CORAL_BLOCK_FALL
WART_BLOCK BLOCK_WART_BLOCK_BREAK BLOCK_WART_BLOCK_STEP BLOCK_WART_BLOCK_PLACE BLOCK_WART_BLOCK_HIT BLOCK_WART_BLOCK_FALL
BONE BLOCK_BONE_BLOCK_BREAK BLOCK_BONE_BLOCK_STEP BLOCK_BONE_BLOCK_PLACE BLOCK_BONE_BLOCK_HIT BLOCK_BONE_BLOCK_FALL
NETHERITE BLOCK_NETHERITE_BLOCK_BREAK BLOCK_NETHERITE_BLOCK_STEP BLOCK_NETHERITE_BLOCK_PLACE BLOCK_NETHERITE_BLOCK_HIT BLOCK_NETHERITE_BLOCK_FALL
AMETHYST_BLOCK BLOCK_AMETHYST_BLOCK_BREAK BLOCK_AMETHYST_BLOCK_STEP BLOCK_AMETHYST_BLOCK_PLACE BLOCK_AMETHYST_BLOCK_HIT BLOCK_AMETHYST_BLOCK_FALL
DRIPSTONE_BLOCK BLOCK_DRIPSTONE_BLOCK_BREAK BLOCK_DRIPSTONE_BLOCK_STEP BLOCK_DRIPSTONE_BLOCK_PLACE BLOCK_DRIPSTONE_BLOCK_HIT BLOCK_DRIPSTONE_BLOCK_FALL
MOSS_BLOCK BLOCK_MOSS_BREAK BLOCK_MOSS_STEP BLOCK_MOSS_PLACE BLOCK_MOSS_HIT BLOCK_MOSS_FALL
liach commented 3 years ago

These emerge as a result of stitch's handling of block.bone_block.break into BLOCK_BONE_BLOCK_BREAK. Imo this propagation logic is fine. I propose to keep it as-is, without adding or removing.

haykam821 commented 3 years ago

@liach The sound event field names are fine, but they are inconsistently reflected in the block sound group field names.

liach commented 3 years ago

My bad! Then imo we should consider this together with #402.

In there, I voted to remove duplicate Block suffix. Here, I feel I am even more compelled to do so given this is a BlockSoundGroup. Which emoji should I leave for "dropped entirely" option?

mounderfod commented 3 years ago

Given that there are fewer blocks with the BLOCK_ prefix, I'd be happy to create a PR which gets rid of the prefix altogether, as from context in #402 it seems that prefixes are not particularly Yarn-like now anyway.

mounderfod commented 3 years ago

Given that there are fewer blocks with the BLOCK_ prefix, I'd be happy to create a PR which gets rid of the prefix altogether, as from context in #402 it seems that prefixes are not particularly Yarn-like now anyway.

I'd also be happy to fix #402 in the same PR if a consensus is reached on both.

Juuxel commented 3 years ago

Given that there are fewer blocks with the BLOCK_ prefix, I'd be happy to create a PR which gets rid of the prefix altogether, as from context in #402 it seems that prefixes are not particularly Yarn-like now anyway.

@redcreeper14385 But we're not talking about prefixes, though? The BLOCK_ prefixes are autogenerated by Stitch and match the IDs (so just fine), but the _BLOCK suffixes in sound groups are just useless.