Skip to content
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

Fix for label not always showing + distance calculation wrong for diagonals #343

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Valerionn
Copy link

This PR fixes two issues:

if (this.labels.children.length > segments.length) {
  this.labels.removeChildren(segments.length).forEach(c => c.destroy());
}

from ruler.js that was added by @TPNils in #329 . There was no explanation why this is needed - and removing it fixes the problem of the label disappearing with not apparent side effects.

  • v12 not following system configured diagonal rule [5e] #342
    canvas.grid.measureDistances seems to return a wrong distance for diagonals in Foundry v12. There is a deprecation warning suggesting to use canvas.grid.measurePath instead, so I rewrote it to this. This fixes the problem - though the options are no longer relevant and I didn't test gridless stuff etc. because it's not needed for our game.

Additionally, I've updated some calls so that no more deprecation warnings get printed when dragging the ruler (except when routinglib is active).

If you need some small adjustments, just tell me - but overall the Foundry development experiment with the de-facto non-existing developer documentation didn't really encourage me to do more testing than "this works in exactly my case and that's good enough for me".

Fix manuelVo#342 distance calculation wrong for diagonals
@AshnakAGQ
Copy link

@manuelVo Please pull. this issue is annoying.

@smeeth123
Copy link

Please pull. Pythagoras is rolling over in his grave.

@SilvesTheRog
Copy link

@manuelVo Please pull

@Valerionn
Copy link
Author

Valerionn commented Sep 11, 2024

@AshnakAGQ @smeeth123 @SilvesTheRog I've made my fork available for install until this PR is merged. You can install it by clicking on "Install Module" in Foundry and then manually copy/paste the following Manifest URL in the input field at the bottom:

https://raw.githubusercontent.com/Valerionn/foundryvtt-drag-ruler/master/module.json

This should replace your drag ruler version. If everything went right, you should see a module named "Drag Ruler [Fork - Valerionn]" with version "1.14.2f":
dragrulerfork

This should work completely out-of-the box but feel free to create an issue over at https://github.com/Valerionn/foundryvtt-drag-ruler if something doesn't work (though I can't promise support for all issues since I didn't create the module and just changed some small things).

I will update the manifest as soon as the original module here is updated so that you will automatically switch back to the original module when you update (but you can also always re-install the original package which should replace the fork).

Also, please remember that people develop these foundry modules for fun in their free time, so please don't spam the PR, I'm sure they'll look at it and merge it as soon as they have time & energy for this (even though I can understand that this issue is really annoying).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants