-
Notifications
You must be signed in to change notification settings - Fork 10
allow jumping to unexplored areas (actually tested) #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lgtm so far, but i would rather add a setting to enable jumping into the unexplored map-parts, do you want to do that or should i do it in a follow-up PR? |
I think so |
? |
@The4codeblocks in case you want it fast then better just add that configuration here with this PR and keep jumping to unexplored areas disabled by default to keep both behavior and max safety. |
done |
Besides config naming comments settings could be moved here: Lines 2 to 20 in bb66479
But don't think that's super important, but if you happen to have a little bit extra time to move stuff. Should this be tested on actual server (thinking about pandorabox test server)? I bet that could be arranged too and some players might be willing to test it there too. That said seems simple enough to me to possibly just get merged without extensive testing... and now that there's disabled by default config IMO rate limiters can wait for another PR. |
|
Co-authored-by: SX <50966843+S-S-X@users.noreply.github.com>
Co-authored-by: SX <50966843+S-S-X@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me but didn't actually test it.
Should be safe too as it now requires explicit configuration to enable jd mapgen.
hm? |
also adds better error handling by displaying every error/warning where applicable