
[1.20.1 (Potentially Higher)] When Damage Types are added via Data Pack, crash ensues when that damage type is referenced in a Damage Source
Closed this issue ยท 26 comments
When this mod is installed, other mods which have added Damage Types via Data Packs, then call them as part of a DamageSource, seem to crash when said DamageSource is called.
I've looked through your source code trying to find the source of this issue, and suspect it may occur somewhere within your onEntityDamage event.
Some details of this:
- Issue occurs only when Dummy mod is installed.
- Crashes occur whenever entity is damaged by the damage source.
- You have addressed this issue with other mods before, but I believe you misdiagnosed the issue as the other mod's fault and should take a second look [1] [2].
Again, the mods in question seem to function fine without this mod installed. I could very well be wrong and many mods are simply creating their damage types incorrectly- but in such a way that functions fine until this mod is installed. If that turns out to be the case, feel free to correct me.
You didn't say which version.
The mod crashes when if finds an unregistered damage type which in the past happened and was other mods fault
1.20.1, as stated in title.
Upon further investigation, it doesn't appear the mods' damage types are unregistered- rather their method's of accessing the registered damage type are incorrect. The method, however incorrect, appears to work without this mod but not with it.
Alas, as a solution has been found, I will let the authors of the broken mods know how to fix the issue and close the one on here.
yeah so might work until a mod tries to send stuff over network or similar. thats usually how it goes for unregistered stuff or issues in general. game can be left in an invalid state and only crash or have issues (like the damage not being sent which would definitly be the case in the case of missing ids) when mods do stuff wrong. Issues with those should be reported to those mods.
My mod simply calls damageSource.getTypeHolder().unwrapKey(). If mods were to throw around unbound holders (unregistered stuff) then stuff like this would indeed happen.
These issues likely arise to 1.20 where the switch to data driven damage sources was done. Saving damages to some static field or such could indeed cause similar issues. When it comes to datapack registries one has to be really careful
Is using damageSource.getTypeHolder().value() somewhat better ?
You should create a resource key for your damage type, like:
public static final ResourceKey<DamageType> DAMAGE_TYPE = ResourceKey.create(Registries.DAMAGE_TYPE, ResourceLocation.fromNamespaceAndPath(MOD_ID, "my_damage_type"));
Then reference it like level.registryAccess().registryOrThrow(Registries.DAMAGE_TYPE).getHolder(DAMAGE_TYPE).orElseThrow()
Avoid any usage of holder.direct()
Yeah I meant of Dummmy's side
What you suggest is already what I do since the first damage type implementation on LSO:
https://github.com/sfiomn/LegendarySurvivalOverhaul/blob/1.20.1/src/main/java/sfiomn/legendarysurvivaloverhaul/util/DamageSourceUtil.java
And the registration is done via datapack :
https://github.com/sfiomn/LegendarySurvivalOverhaul/tree/1.20.1/src/generated/resources/data/legendarysurvivaloverhaul/damage_type
If you find another way of register / call the damage sources in order to make Dummmy's mod work, I'd gladly take it.
So far, the way it has been implemented is the standard way, and I failed to reproduce the crash, so I don't know what's going on
I don't know, never been able to reproduce it while from time to time, a ticket mentions it ...
Even when using the same LSO version as the one in some crash logs such as in this ticket (#36) couldn't reproduce it,
also idk if its related or not but i have not used that bootstarp context code before when making new damage types. Just wanted to mention that. From what i can see everything else is how i would do too, nothing wrong. the fact that it only happens with some other mod could indicate that some other mod might be interfering, maybe throuh that bootrastap context code by firing it early or something ? idk
The bootstrap code is just there to be called by the DataGen event, it creates the damage types JSON files trough code
And from what I can see, the generated JSON files look correct for damage types
So yeah, truly a mystery this crash ...
ah i see. yeah idkthen. clearly some mod is messing with damage types. as to why it gets reported here often? not a lot of mods check for damage types ids and dummy can add numbers for player damage taken and legendary survival adds some damage types that damage the player very often with a new damate type so its just very likely for it to happen with those 2.
Is it possible to just silently ignore the error if damage types not found ?
Like either not showing the damage, or showing it with a default color or idk ?
ah i see. yeah idkthen. clearly some mod is messing with damage types. as to why it gets reported here often? not a lot of mods check for damage types ids and dummy can add numbers for player damage taken and legendary survival adds some damage types that damage the player very often with a new damate type so its just very likely for it to happen with those 2.
It's not the dummy nessisaraly @MehVahdJukaar
This damage type in my play with it was never inflicted on the dummy; BUT other mobs inflicting it onto other mobs (not the dummy, and through a enchantment by a third-party mod that inflicts debuffs (the damage type being reported comes from an effect))
For some reason though when this happens (let's say a skeleton inflicts the effect they caused this damage type to be inflicted on a zombie) it seems that when the crash happens that Dummmy is picking it up for some reason (either because how it's coded or it's a frequent false-positive being flagged when making the crash report (points to the wrong mod))
well it could point to the wrong mod but that crash only happens when a mod has incorrectly aded a damage type, likely missing their json file so issue is still there
It's when you use Holder.direct
for the damage type passed to hurt
direct holders have no key you can access (it's null)
I don't know if this is really a bug / something you shouldn't do
It would be cleaner to add a data part for this damage type (this can technically be done through a datapack by the user)
Personally i'd recommend never throwing exceptions unless it genuinely breaks the game
In this case you might want to return a generic fallback damage type or just skip displaying it
Yup that's precisely the issue. A holder direct is an unregistered one. You should never use it and add it's JSON properly instead
Generally it's good convention to hard fail where incorrect state is detected to so errors can be found. And I confirm that it's their issue. The damage type doesn't have a JSON it seems and direct holders should never be used for anything really. Use proper bounded holders like the game dies everywhere else, surely for all damage types
I would not exactly call it an incorrect state
It seems to cause no issues in the base game and most mods
In my opinion in a modded environment with a varying quality of mods you should be as defensive as possible and avoid crashing the game as much as possible
The experience can be unstable enough, no need to make it worse when it's not needed
There are many things that would cause no issues until queried. For example you could have a method override for a non nullable method return null. Until that's called, nothing will happen. It's still an incorrect state as it's breaking a method contract. This is 100% their issue, I double checked with other modders too. If it was my mod I would have appreciated this because now we have a report that shows an issue they have and you can report it to them
We're not doing super clean coding here
We're trying to work in an unstable modding environment due to varying degrees of modding experience
And the end user has to deal with the crashes
I'm not saying "catch everything" but in this case at least you really don't need to throw around exceptions
Either way I'm not related to either mod so I'll leave it at that
Did you report it there? Damage types not added to datapacks are part of a sloppy 1.20 port, 100% incorrect and undoubtedly a mistake. The way I see if reports like these make mods a favour since they make their issues discoverable. As for your last statement there's no difference between catch everything and only catch some things. How would I even know what dumb thing people might do beforehand and which I should fail gracefully and which not. Besides if I wer to fail gracefully a part of the mod here will not work with no user feedback making the user feel my mod is at fault