Inheritance Array Favors First Indices?
Ameranth opened this issue · 10 comments
I've been fiddling with your dungeon settings and, from my testing, it seems that for example:
{
"name" : "dungeon:mesa_builtin",
"exclusive": true,
"inherit" : [
"builtin:mesa",
"layout:size_small"
]
}
This code uses the layout sizings of builtin:mesa
, not layout:size_small
. The desired outcome is achieved by listing layout:size_small
before builtin:mesa
. I apologize if this is not the case on your end, but I hope this can help you in some way.
Thank you for your time and work!
Seriously, thanks for this issue.
I think this is a bug I've introduced while refactoring in the past.
This issue had the capacity to keep me up all night, so I've found what appears to be the root cause.
Unfortunately, I'm in the middle of a big change for 1.14+, so I'm not sure when I'll have the confidence to do the next release. Maybe soon!
Would you be open to beta testing a build with the fix, @Ameranth?
Awesome, I'm glad this was helpful to you!
I could certainly confirm a 1.12 beta fix for you, though I won't be able to get to it until some time tomorrow :S
Best of luck with the 1.14 port, many will be ecstatic to have it!
Once again, thanks for your attentive eye and the time you took to report this. Community involvement like yours is what gives me the drive to keep at this project.
This (beta) release is all thanks to you. You can download it from here and play it at your leisure. Please let me know if you find any issues.
https://github.com/srwaggon/minecraft-roguelike/releases/tag/V2.2.1
@Ameranth Thank you for this interesting issue submission.
I've tested this and it appears to be true. I find this to be surprising behaviour.
Intuitively, my instinct is that later inherits should take precedence, and this behaviour betrays that.
I don't know if this is intended behaviour from the original mod, or something that I've introduced through refactoring.
If I change this behaviour, I'll be curious how this could affect long term users.
I'll test with the original mod and provide more information when I can.
As a follow up, I tested using themes.
This might have been a bad test, as overrides exist and are not very straightforward.
https://github.com/srwaggon/minecraft-roguelike/wiki/Overrides
Any DungeonSetting.. field.. that is overridable seems to favour the parent(/previously inherited) setting, unless it is specified in the overrides array (in which case it then favours the child/inherited setting). I'm very unsure why this decision was made. It leads me to believe that the behaviour that we're seeing is intended.
When you witnessed this behaviour, was it specifically with the level settings that you provided in your example?
If so, I should note that the small level settings are very similar to the defaults, and I personally have difficulty comparing them.
Thanks for looking into this!
Yes I agree that this behavior is unintuitive; inheritance conceptually favors the highest-level data, as otherwise there is no point in specifying data deviating from the inherited to begin with. However if the code suggests that the behavior is intended, it's likely more an issue of the feature being improperly named as "inherit."
That being said, if you're worried that longtime users will have issues with a fix, probably simply adding a note on this to documentation would be sufficient.
I noticed this problem while trying to reduce room count significantly (counts are at 7 rooms in XL layouts, down to 2 rooms in tiny layouts--the difference is quite stark with these values), and found that none responded; I used dungeon:mesa_builtin
as a test case to report the issue.
Thank you for your kind words, I'm familiar with various stages of "dev drive" as well, so I'm very glad to hear that :)
As to the beta, I gave it a shot but there were a few problems:
-
It rejected some of my loot settings .json files with the error "
Failure:Error in: loot_xx.json : An unknown error occurred while parsing json: class java.lang.NullPointerException null
".
After some testing, I found that this happens to loot file which defines loot with"type": "potion"
. -
After working-around the above issue for the sake of the test, I tried to spawn a dungeon with
/roguelike dungeon nearby dungeon:cave
but was met with this url:
which seems unintended. I understand the error was because I had removed loot files thatloot_modded.json
was still referencing, but the/roguelike settings reload
command--which I did before generating the dungeon--should have caught the error I think. -
Lastly I reworked some dungeon files to respect inheritance ordering, and this did seem to work for rooms and roomcounts, however I was unable to get a tower to work, regardless of where I placed it in the inheritance order. In fact, this dungeon code was generating with no tower at all (not even a "hole" type) when specifying one. Removing the tower defaulted back to the rogue type tower, however.
{
"name": "dungeon:cave",
"exclusive": true,
"inherit": [
"dungeon:common",
"demo:rooms",
"tower:rogue_forest",
"layout:size_small",
"theme:cave",
"filter:muddy",
"filter:viney"
]
}
In response to the NullPointerException
, I've fixed that too. Another excellent find, and thank you again for your excellent communication and documentation.
c33e6f0
The new .jar has been uploaded to the v2.2.1 (alpha) release to replace yesterday's .jar.
The issue was that I had written code that assumed that all potions loot items had "forms" (regular, lingering, splash), when in reality this field is optional and defaults to regular if absent.
I've also done a little testing with the hole
tower and I'm unsure what is supposed to happen with it. I have a memory of it creating a pit that players could stumble upon, completely lacking a staircase. In my recent tests, and looking at historic code, this does not seem to be the case.
The code suggests that this clears an area above the ground where the tower could spawn, but does not produce a pit. I might choose to change that in the future and make it be a pit.
Great work!
From what I can tell, things are in order now, apart from the hole
tower--which you were right, hole
seems to be the only tower that isn't working, the others seem fine.
Indeed hole
is, in version 1.12.2-2.1.4
, creating a pit as you recall (it's very nice for hiding smaller dungeons!). Here's what they're looking like on my end:
Generated by this dungeon json, in 1.12.2-2.1.4
:
{
"name": "dungeon:cave",
"exclusive": true,
"inherit": [
"layout:size_small",
"tower:hole",
"theme:cave",
"filter:muddy",
"filter:viney",
"demo:rooms",
"dungeon:common"
]
}