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

Issues with ZoomFunction #649

Closed
Merudo opened this issue Sep 2, 2019 · 5 comments
Closed

Issues with ZoomFunction #649

Merudo opened this issue Sep 2, 2019 · 5 comments
Assignees
Labels
feature Adding functionality that adds value macro changes This issue adds or changes macro functions. Extra work is required (testing, wiki, code editor) tested This issue has been QA tested by someone other than the developer.

Comments

@Merudo
Copy link
Member

Merudo commented Sep 2, 2019

Is your feature request related to a problem? Please describe.

  • The functions getZoom, setZoom, getViewArea, setViewArea, and getViewCenter can be changed to use the consolidated methods in FunctionUtil
  • Functions getZoom and setViewArea are not displaying errors when given too many parameters
  • The javadocs have errors
  • There is an extra delimiter at the end of getViewCenter

Describe the solution you'd like
Fix those issues.

@Phergus Phergus added the feature Adding functionality that adds value label Sep 3, 2019
@Phergus
Copy link
Contributor

Phergus commented Sep 19, 2019

It appears that you have changed the 2nd set of return values of getViewArea() to be end points and not the width/height as it was previously. Was that intentional? I don't recall any discussion on this though that doesn't mean it didn't happen. ;)

Current Code

Pixels:  startX=1,startY=0,endX=1291,endY=868
Cells:   startX=0,startY=0,endX=25,endY=17
Zoom:  1.0

Pixels:  startX=401,startY=401,endX=1691,endY=1269
Cells:   startX=8,startY=8,endX=33,endY=25
Zoom:  1.0

MT 1.5.1

Pixels:  offsetX=0;offsetY=1;width=1290;height=893
Cells:   offsetX=0;offsetY=0;width=25;height=17
Zoom:  1.0

Pixels:  offsetX=400;offsetY=400;width=1290;height=893
Cells:   offsetX=8;offsetY=8;width=25;height=17
Zoom:  1.0

Also, getViewArea() appears to be returning the pixel coordinates in zoomed map pixels? Is that actually useful for anything? Need actual map pixels for moveToken() and goto() only takes cell coordinates. If there is a use case for that, it should probably be documented on the wiki.

I was expecting the pixels to be either straight map pixels or display pixels. Maybe the later would be a separate function like getDisplayArea() which would return the width/height of the map display area in screen pixels.

setViewArea([r: "51,51,550,375,1"])<br>
[h: setViewArea(51,51,550,375,1)]
Pixels: [r: getViewArea(1)]<br>
Cells : [r: getViewArea(0)]<br>
Zoom: [r: getZoom()]

Returns:

setViewArea(51,51,550,375,1)
Pixels:   startX=131,startY=117,endX=1421,endY=985
Cells :   startX=1,startY=0,endX=10,endY=7
Zoom:  2.585170340681363

The values for Cells match the actual display but should, for a 50px grid, be (1,1) instead of (1,0). So the map isn't positioned quite right.

@Merudo
Copy link
Member Author

Merudo commented Sep 19, 2019

I did most/all of the changes you mentioned were made prior to this? They are present in 1.5.3, which is before any of my code got integrated into Maptool.

@Phergus
Copy link
Contributor

Phergus commented Sep 19, 2019

Yes, the change from width/height was apparently made after the initial addition of the function for 1.5.1 and without any mention of why it was done in the issue or in the PR that made the change.

I'm bringing it up now because I'm actually testing the functions and you're the lucky coder who just touched them all. :)

So working as designed and the question becomes is the design useful?

@Merudo
Copy link
Member Author

Merudo commented Sep 19, 2019

I will open an issue.

@Phergus Phergus added macro changes This issue adds or changes macro functions. Extra work is required (testing, wiki, code editor) tested This issue has been QA tested by someone other than the developer. labels Sep 19, 2019
@Phergus
Copy link
Contributor

Phergus commented Sep 19, 2019

Tested and all functions working as designed/expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value macro changes This issue adds or changes macro functions. Extra work is required (testing, wiki, code editor) tested This issue has been QA tested by someone other than the developer.
Projects
None yet
Development

No branches or pull requests

2 participants