Geitenijs / KeepChunks

The most powerful chunk loading plugin!
Other
42 stars 9 forks source link

Fixed totalRails counting error. #25

Closed dorrzun closed 4 years ago

dorrzun commented 4 years ago

Resolved totalRails count being incredibly wrong. I looked into using the Block class for a rewrite, but it just makes more sense to truncate/normalize the rogue value via the setter.

Geitenijs commented 4 years ago

I've been testing it and the patch doesn't seem to fix the issue. Track has 43 rails, but it never actually says 43

Have a look: https://postimg.cc/gallery/9QbB4Mf

dorrzun commented 4 years ago

Should be all fixed now with the latest commit! There was a missing check in the getNSEW() functions, allowing duplicates. Not sure when that appeared in the source code, but has been there since the beginning.

Let me know if the issues (somehow) still persist!

Geitenijs commented 4 years ago

I see!

The count has been fixed and is finally consistent! Downside is that the command no longer marks/releases chunks for me ;)

dorrzun commented 4 years ago

Hahaha oh the wonders of debugging! Take a look at the latest commit, it's now working again for me :)

Geitenijs commented 4 years ago

It actually finds the right chunks but it somehow doesn't load them...

Screenshot
dorrzun commented 4 years ago

Uh oh! I’m not sure where the issue might be for that...where to start?

Geitenijs commented 4 years ago

Not sure, don't have time to really look into it at the moment. Will do soon. If you find anything, please let me know!

dorrzun commented 4 years ago

Of course! If there are any crash course tips for debugging this issue, let me know. I’ll work on this today and see where I get!

dorrzun commented 4 years ago

I believe I have found the "error"! It is a false negative, and is most likely a result of bad configuration on both of our ends.

To keep it simple, my usual testing routine during plugin development is as follows:

  1. Make changes/recompile/build artifacts
  2. Run shell script to remove old plugins/KeepChunks directory and replace with the new .JAR
  3. Run /reload in game.

The "bug" arises when I carelessly run the above procedure while there are still marked chunks being tracked by KeepChunks. If this happens, the newly unpacked plugin is not tracking the old marked chunks (because the data.yml is fresh and newly created).

This explains why chunks were remaining force-loaded despite them not being marked by KC. Secondly, because those chunks are still force loaded in game, the isChunkForceLoaded() condition now ALWAYS fails (Line 178/183 in KeepRail/ReleaseRail). That explains why you weren't seeing the "Marked Chunk" debug message at all :)

If you are still having errors, be sure to run /forceload remove all, to verify there are no unkempt chunks in memory.

After doing this, I was able to run the same code from this latest commit with no counting or chunk loading errors.

Geitenijs, I do apologize for writing a technical essay on all of this, I just want to make sure that my thought process is detailed and to verify that we are both on the same page. :)

Geitenijs commented 4 years ago

Oh wow! When I saw the notification email for your initial comment, I quickly reviewed the code and figured that had to be the issue. Thank you for looking into it as well, really appreciate your work!

Will address tomorrow, just need to figure out how to properly sync the forceload & KC list