Migrate Neoforge damage type tags to convention tags

[muon-rw]

In the spirit of easier cross-platform compatibility, migrate #neoforge:is_magic to #c:is_magic, for example.

Fabric does not yet implement any damage type convention tags, as per the issue just submitted here: FabricMC/fabric#4084

[TelepathicGrunt]

Actually, careful here. Neoforge actually does bind behaviors to damage tags. In fact, poison was unhooked from vanilla magic tag to neoforge’s poison tag. So some of these aren’t true convention tags but actually gameplay behavior tied/changed vanilla mechanics

[muon-rw]

I guess that explains why this hasn’t already been implemented. Not sure how to proceed in that case

[TelepathicGrunt]

I believe it is only the magic and poison damage type tags that can’t be convention (due to moving vanilla poison damaging from magic to poison)

you’ll have to handle magic/poison separately per loader to get the behavior you want. The other tags may be ok to be under c but would need someone to recheck if those have any behavior attached.

Cc: @Shadows-of-Fire

[Shadows-of-Fire]

We could maybe add bouncer tags that just include the platform implemented tag? That seems a bit silly though.

[dhyces]

Could add them as c tags and then use the c tags in the neoforge impl tags

[TelepathicGrunt]

What Neoforge specifically did was created a whole new DamageType called poison. Then made the Poison mob effect use that instead of the Magic DamageType. Then put magic under neoforge:is_magic tag and then the new poison underneoforge:is_poison tag.

This means that if we add c:is_magic and c:is_poison, either Neoforge puts the poison damage type into the magic c tag to make the poison mob effect trigger the same c tag as it would on Fabric. But now Neoforge modders would be confused as to why poison is tagged as magic and will make bug report. It also means any modder using the poison damagetype that neoforge provided will now be considered c:is_magic instead of their intended c:is_poison tag. Causing a mess with modded usage.

If we decide to have Neoforge's poison damage type in c:is_poison instead, now the magic and poison tag is actually different between Fabric and Neoforge as the poison mob effect will trigger the other tag on Neoforge. Defeating the entire point of the c tags.

I do not think magic and poison should be under c because they fundamentally behave differently on Fabric and Neoforge. And I doubt Fabric will create a poison damage type to mimic Neoforge.

The other damage type tags can go under c. Just not magic and poison imo.

Original PRs for reference:
#59
#509

[VoidLeech]

This means that if we add c:is_magic and c:is_poison, either Neoforge puts the poison damage type into the magic c tag to make the poison mob effect trigger the same c tag as it would on Fabric. But now Neoforge modders would be confused as to why poison is tagged as magic and will make bug report. It also means any modder using the poison damagetype that neoforge provided will now be considered c:is_magic instead of their intended c:is_poison tag.

The neoforge:is_poison tag is currently also included in neoforge:is_magic, so unless that's a bug, I don't see where the Neoforge modder confusion would come from if the same was mirrored /w c:is_magic and c:is_poison tags.

NeoForge/src/generated/resources/data/neoforge/tags/damage_type/is_magic.json

Line 7 in bdda6d2

"#neoforge:is_poison",

[TelepathicGrunt]

@Shadows-of-Fire why is poison in magic : screm

I wouldn’t consider poison as magic if it is being split up unless we consider snakes as mythical creatures lol

[Shadows-of-Fire]

Because Vanilla uses magic damage originally, so to maintain behavior poision is magic

Magic actually carries the damage type semantics (i.e. magic bypasses armor) - poison is just informative and allows mods to react to poision specifically

[TelepathicGrunt]

The original pr for the split actually broke that behavior for poison. And it was restored in this pr by adding poison damage type to the same tags that magic was in. So there isn’t any need to put poison in the neo magic tag when the behavior was controlled by the other various minecraft tags #1215

the neo magic tag doesn’t control Mc behaviors

[Shadows-of-Fire]

That fix is probably wrong then, magic should be the one included for behaviors, and magic must include poison (because poision is originally DamageSources#magic())