Ice fill solver issues

[past widget]

• Sometimes completely breaks and doesn't show the red lines (most of the time it shows for first and second ice fill section, but the last one [one where chest with blessing is and puzzle is finished] takes a while to appear), waiting a while it eventually solves and displays the red lines but this takes some time (talking about >15 seconds of time) I believe this is directly related to #2

• Both solver and rendering performance seems to be suboptimal, getting lower fps after entering the room when bug #1 happens till the solver finishes, and then itll be back to decent performance but still lower fps than before entering the room, probably cause even if it solved the puzzle it still has to render the red lines.

I believe the poor solver performance is probably caused by using recursion instead of a loop (or tailrec function, but the function seems to execute 2 other functions after calling itself, which makes it regular recursion instead of tail recursion) here: https://github.com/Skytils/SkytilsMod/blob/1.x/src/main/kotlin/gg/skytils/skytilsmod/features/impl/dungeons/solvers/IceFillSolver.kt#L197

Don't know if it can easily be turned into a loop or not but if it could be I believe this would solve the performance issue.

Do note that not all ice fill variants trigger this issue, and how long it takes to solve might also depend on hardware specs.

• Keeps rendering even if out of the room/puzzle finished, causing lower fps in all of the run (sometimes I can even see the red lines from boss room before chunks get loaded) than usual till you hop into a new dungeon/change world (I believe this is caused by it only resetting the variables on WorldEvent.Load instead of after puzzle finish or room leave and then unconditionally [except if config option is disabled] rendering the lines if the variables aren't null?)

[past widget]

Thought maybe some screenshots would help so here's some. First image is while it's solving (low FPS). Second one is after it solved it (FPS goes higher, but still lower than before first entering room), Third one showcases that it still renders even behind blocks but is invisible unless you spam F3 + A to see it (You can even see it in boss before chunks load even without needing F3 + A if you play with a low view distance.)

As I said, turning the recursive implementation into a loop or tailrec function along with not rendering unless in the room or only rendering if the puzzle is not finished would be a good fix.

Note: Cropped irrelevant stuff on screenshots, let me know if any more information is needed. Sorry for the fire particles; my grand searing rune got into my view while taking the screenshots.

[flat crest]

Can reproduce the last section taking a long time

[vital walrus]

i get this aswell

he may be right about why

[flat crest]

I accidentally fixed it so it should be in the next release

[vital walrus]

lol accidently

wish i could fix stuff by accident like that

[flat crest]

Simply be bad at what you're trying to do!

[flat crest]

While the performance issues and unreliability should be fixed next release, I forgot to add the puzzle completed check 💀 (So it'll still render even when it's done, will probably be fixed once it gets rewritten by me or a Skytils dev)

[past widget]

I think that got fixed in a commit before your pull request, so we should be fine? https://github.com/Skytils/SkytilsMod/commit/5b6d81f6b0ffd2052df87f1d55736079b2b66448

[past widget]

If you want to help confirm it's fixed (since the puzzle patterns are random and this issue didn't happen on all variants) you can download a nightly build of Skytils with the fix from this link: https://github.com/Skytils/SkytilsMod/actions/runs/8402378087 (click on build-artifacts, unzip it, place the Skytils jar without -dev at the end to the mods folder. Make sure about placing the correct jar as the one with -dev only works in a development environment.)

Disclaimer: Update to the actual release as soon as it gets released as these jar files generated by GitHub Actions automatically might not be as stable, in addition to that this currently a build based on 1.9.0-pre6 so if you are on 1.8.x stable I suggest reading changelog of all the pre-releases, and it will be less stable than 1.8.x (Stable in this context meaning it can crash or have new bugs due to the new code introduced [even bug fixes can introduce additiional bugs] but otherwise will have more bug fixes and features than the 1.8.x).

[flat crest]

epic!

Also I think 1 variant is still broken sometimes

[past widget]

I did not have time to test it thoroughly yet, but I will update about it here if i still get the issue

[past widget]

With Skytils GitHub Actions build I've mentioned earlier, this variant took like a solid 20 seconds to solve (Or at least felt like it, not sure about exact time it took. This was for all 3 parts, but guessing the issue is still on the last part and all of them appearing at same time is a result of using Triple instead of seperate vars for solver).

Was also rendering after completion but after checking commits I can see a new commit that fixes this regression, I will update to build from that commit and check again later when I have time, and I will update here if the fix works.

Thanks for the work on this issue so far!

[flat crest]

I'm working on a "better" version rn

Ig I could do a pr so it can be tested?

[past widget]

Sure

[flat crest]

https://github.com/Skytils/SkytilsMod/actions/runs/8451249313/job/23149175474?pr=478

Just gotta wait for it to finish

[past widget]

Alright, will update here when I test it

[flat crest]

I just realized a mistake

But ig it's fine

[flat crest]

If it doesn't start the solver, you gotta get like more than 25 Blocks away from the ice fill chest

[past widget]

To reload the chunk with chest, or something else?

Will keep in mind though, thank you

[flat crest]

I accidentally inverted a check so it only scans chests far away

[flat crest]

I might've completely broken it or I'm just unlucky and got the variant that doesn't work

honestly, fuck it, imma just replicate the ice fill room

Okay yep mb inverted another check

[flat crest]

For some reason that takes so long, ig mainly cause of the amount of spaces?

[flat crest]

Best algorithm!!!

[past widget]

Might worth attaching a profiler like visualvm to see which method takes the most time, although 1 second is acceptable, it becomes an issue above like 10-15 seconds for me

[flat crest]

I know that it's the path recursion stuff, but I don't really think there's much to improve except maybe parallelization

(Since I also want the straighest path and not only a path)

tho that's a massive increase for only a slight increase in paths so maybe there's some issue I'm missing

[past widget]

Does this mean the function recursed itself 32 times? Maybe its not the recursion then, not sure

[flat crest]

It recursed itself probably like ~1000 times and found 32 paths

Actually probably way more than 1000

[past widget]

In that case making the function tailrec might help but it runs 2 other statements after calling itself so thats incompatible with tailrec in kotlin

Parallelization would be complex for thread safety and would only cut the time to the amount of cpu cores so that's not really a good solution i think

[flat crest]

got an idea finally

[flat crest]

4282893 damn

Looks about right

Amazing optimization, 1/4th of the iterations now

[past widget]

Good work! Might be enough to make it not take annoyingly long, I could test in a few hours if you push to github, I don't think I can test a lot of variants though but you could try the variants on my screenshots I sent above that took long on your replicate of ice fill

[flat crest]

I'll try to finish this up in like the next 2 hours and then pray it works in dungeons as well xd

[flat crest]

https://github.com/Skytils/SkytilsMod/actions/runs/8457164800?pr=478

[flat crest]

https://github.com/Skytils/SkytilsMod/actions/runs/8474697860?pr=478

latest version

[past widget]

Sorry for the late reply, I have been busy before and after that I was trying to find a variant that would take long. Tested it for a few days and I can now say that it works fine.

Wasn't able to find a variant that took long yet, I will update here if i find any.

Only didn't work 1 time partially but I think that was my fault, I gone to the room with 500 speed and fall down on the first section before it solved, and as you might know the ice blocks disappear when you fall down (step on the wrong block), if the solver triggers/finishes while ice blocks aren't there it will skip that section.

Likely a bug not worth fixing and would happen before your PR on the original solver implementation as well though, but I'm attaching a screenshot just in case.

The screenshot is after the ice blocks came back (after falling) and I finished the first section. I did not test if re-entering the room would retrigger and fix the solver as I did not think of it at the time.

I have also gone ahead and turned the ?: emptyList() sections you have commented into !! and built the PR on top of latest dev branch (based on 1.9.2) and tested about 3 ice fill rooms and it seems to work fine, so I don't understand why wouldn't you want to use !! there if you are sure it can't happen. No idea about other todo comments though.

[flat crest]

I just hate !! cause it assumes that my logic there is perfect and won't break (which it definitely will cause I suck at coding), but in the latest non-pushed version I have I also opted for !!

Also it breaks cause yea it only checks once a bit after first entering the room, so if you break it and then it tries to solve it just stays stuck, but as you said not worth fixing cause it can only happen on the first section

[flat crest]

Btw did you also test the "optimal path" thing? (For that you need to click on Dungeon Sweat Mode in the config), it's slower, but will find the straightest path

[past widget]

Oh I did not see that. I will test it and update here, had no idea it was a thing honestly, I saw that you had 2 different methods but I just thought it was for like testing which one is faster. Will write if i get any issues or in a few days if i don't get any issues.

[flat crest]

Cool, thank you!

[past widget]

I think it works fine, tested with optimal path as well. Didn't get any variants that took longer than 3 seconds to solve yet. Never crashed or said unable to find solution.

Can be merged to be tested by more mass user base in the next release maybe, unless you have other unfinished stuff left to do?

[flat crest]

While I have some more TODOs in there, I think it's fine, now hoping it gets merged fast

[frail dune]

there are only 4 variants for ice fill - and only one of them takes like 4 seconds for your solver to find the correct solution

[flat crest]

wtf I see you everywhere I go now

[frail dune]

I'm not in your walls

[flat crest]

That's exactly what someone in my walls would say

[frail dune]

(yet)

[past widget]

Can you update the PR to work with the new ice fill room? I was using a build from your Pull Request till ice fill room got updated and it was working flawlessly, now I have to use the regular Skytils version and its really annoying when it takes a lot of time to solve.

[flat crest]

Oh yea I forgot about this

I don't think I have a Skytils project set up, so you're probably gonna have to wait until I get motivation to do it

[dim moat]

✨ motivation ✨

[flat crest]

btw, did they add any new layouts or anything?

[flat crest]

By that, I'm assuming the regular skytils one still works perfectly fine for all variants and such right?

It's just slow?

[dim moat]

I think they did

[flat crest]

apparently it's not happening today

[dim moat]

:intimidated:

[flat crest]

like this should've worked so idk why it doesn't ;-;

[flat crest]

Finally, Skytils

[flat crest]

Actually this might've been easier than I thought

Okay so, I think I'm done, however I cannot find a Ice Fill to test it ;-;

[flat crest]

Cool it works

[past widget]

Nice work! Small note though one of the chests currently doesn't appear (the place for it still is there just the chest isn't) in the real room to my knowledge so hopefully that's not an issue

I can start testing it if you update the PR with latest source.

[flat crest]

yep not an issue

I made 1 change, imma quickly check whether that broke everything, and if it didn't imma push

Okay good that I tested cause it didn't work

[past widget]

PR seems to be closed and shows 0 file changed, are you going to open a new one?

[flat crest]

yep, cause I did it on a diff branch and couldn't be bothered figuring it out

Pull Request

Download Link

Stuff I should prob do:

• if optimal fails (for whatever reason) try the first path function instead
• set a limit of like 10s? for optimal, if it takes longer it should just fail

[frail dune]

is making a static solution set still not feasible?

[flat crest]

I personally just can't be bothered to do it ngl

Also idk what all variants are

(And it's like 100ms for all 3 solutions with the fast function, so it's not really that big of an issue)

[
  [true, false, false],
  [false, false, false],
  [false, false, true]
]

[
  [-1, 6, 5],
  [2, 3, 4 ],
  [1, 0, -1]
]

is probably what it has to look like if you just want to save all paths

And that's 1 out of like 24 combinations? idk (with like the others being 5x5 and 7x7 as well)

Or well actually you only need one of those two arrays

[frail dune]

I think all of the variants have different amounts of stone / stone in different places

so only need do to 2 checks on 2 blocks to figure out which variant it is

[flat crest]

Well I still need to save the entire path

And since the path will have to be:

[
  [-1, 6, 5],
  [2, 3, 4 ],
  [1, 0, -1]
]

I already know where the stones are (-1)

[frail dune]

couldn't you just store the path as a ternary number

0 being turn left, 1 for forwards, 2 for turn right

[frail dune]

like for this one here

0221002

[flat crest]

ig?

[frail dune]

hm

I will go make a thing when I have some time

[flat crest]

btw if you're gonna make pre-calced paths, use the optimal function for it

[frail dune]

I was just going to manually make straight line solutions for etherwarp

[flat crest]

didn't know you could use etherwarp for it

however technically the optimal function should do the straighest solution

[frail dune]

it's nice for high ping when you direction change

reduced the chance of going diagonally

so what you want in the end is just a bunch of parallel lines

[flat crest]

btw, has anyone tested this and does it work fine?

[past widget]

it works fine

[flat crest]

Also I think imma increase the detection range, cause this doesn't even trigger when super close to it (aka 2 blocks away)

[flat crest]

A faster and higher range version

[past widget]

never had an issue with that but maybe that's just because i just run to the starting ice block when entering the room directly

positive change anyways since if the solver starts earlier on room enter it should finish earlier as well

though, its already fast enough I never had a slow solving issue on both your old and new pr in both modes (optimal and regular) so I think it should be good to be merged

[frail dune]

is there a reason you moved away from chest based detection?

[flat crest]

It is chest based

The thing is just, you need to be within 25 blocks of the chest

[frail dune]

oh its cubed

wait

is it

[flat crest]

ig what's probably a better idea is just checking if the player is in that puzzle room but that's not my job

[frail dune]

might have a go at this one later

and i guess tile entities don't fire the onentityjoin event?

[flat crest]

I don't think you are allowed to use that but idk

Tho idk exactly how hypixel sees that stuff

[frail dune]

uh

[flat crest]

tho ig you can only see the lines if you're in the room anyway so maybe it's fine?

[frail dune]

doesn't the solver only start drawing when the puzzle is "found"

[flat crest]

It also only starts scanning once it's been found

[frail dune]

meh
could just swap those two around

should still be fine

[flat crest]

Probably best solution is check if they're in the room

[frail dune]

I feel like we've had this discussion before for some reason

[flat crest]

probably idk

[frail dune]

update: this is absolute hell
the bit where the dungeon is stored as an array only references room state
and unique rooms don't contain info related to their position
time to figure out how exactly the room name rendered gets info on where the room is in the array

[dim moat]

you need some help or smth?

I won't be able to do shit but I'll still ask anyways

[frail dune]

lol
help would certainly be nice, aa I don't have a clue what I'm doing

[dim moat]

alright well I gotta go play some tunes, once I get back I'll look through and see if I can maybe do anything

[frail dune]

:react_useful:

https://github.com/Kemmisch/SkytilsMod/tree/dev/src/main/kotlin/gg/skytils/skytilsmod/features/impl/dungeons/catlas

[dim moat]

this is catlas...?

[frail dune]

yeah

oh

changes are still on local

ah nevermind, I (think) I found what I was looking for

I'll test it out properly tomorrow

[dim moat]

alright cool

lmk if you need any assistance

ig

[flat crest]

lmk if you need any assistance

ig

[frail dune]

here is thing

[flat crest]

epic!

now is the question how long it'll take for my pr to get accepted so that you can make a pr based on my pr

[frail dune]

idk about making a pr
my code and git history is horribly messy

and probably not fit for going into the actual mod

[mortal tide]

just skid odins

theres is already very good lamo

[flat crest]

It just has a file with the solutions from what I saw

(assuming I looked at the correct project)

[mortal tide]

If you can steal Odins