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.

Sign up for GitHub

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

Jump to bottom

Vision for Non Individual View Broken #357

Closed
Phergus opened this issue Mar 21, 2019 · 11 comments
Closed

Vision for Non Individual View Broken #357

Phergus opened this issue Mar 21, 2019 · 11 comments
Assignees
@Merudo
Labels
bug high High priority bug/enhancement tested This issue has been QA tested by someone other than the developer.

Comments

@Phergus
Copy link
Contributor

Phergus commented Mar 21, 2019 •
edited
Loading

MT Version 1.5.0 - Both installed build and JAR only build with Java 10.
Also seen in Nerps 1.4.4.0, 1.4.5.4 and MT 1.3.b87, 1.3.b89, 1.3.b91, 1.4.0.5.
OS: Win10x64

With a server running and server option Individual View not enabled there are two problems.

First problem is that with Player View enabled on the GM client, no PC or NPC tokens are visible. Selecting a token prior to switching to Player View has no effect on this.

Second problem is that the Player Clients are getting Individual Views and not a shared PC view. With an owned token selected, player clients should get that token's individual view. With no tokens selected the player clients should display the combined view visible to all PC tokens . Related is that a player client with no token ownership does get the all PCs view.

The expected view for Player Clients is what is seen in MapTool with no server running and no connection to a server. With Player View enabled and no tokens selected, all tokens within the viewable area of any PC tokens are shown. With a single token selected, only those tokens within the selected token's viewable area are shown.

VisionProblem1
Top-Left: GM Client with Player View enabled
Top-Right: Player Client with no token ownership
Bottom-Left: Player Client with ownership of Elf token
Bottom-Right: Player Client with ownership of Mystic token

Campaign file used in screenshots.
VisionProblem1.cmpgn.zip
@Phergus Phergus added bug high High priority bug/enhancement labels Mar 26, 2019
@Phergus Phergus mentioned this issue Mar 26, 2019
Closed
@dorpond
Copy link

dorpond commented Apr 28, 2019

I just tested in Nerps 1.4.5.3 and the issue still exists there. I know I worked closely with Jamz on vision and lighting back in Nerps 1.4.4.0, and he eliminated a bunch of bugs, but to be honest, I tested individual views more than not, but nevertheless, I'd like to see if this issue was still in 1.4.4.0. Jamz did some change lighting/Vision in 1.4.5.0, so it will be nice to know if he introduced that bug or if it was there in 1.4.4.0.

Previous Jamz versions can be downloaded here in case anyone wishes to test 1.4.4.0 before I get a chance. It could provide a clue:
https://github.com/JamzTheMan/MapTool/releases

@Phergus
Copy link
Contributor Author

Phergus commented Apr 28, 2019

It's broken exactly the same way in 1.4.4.0. So most likely this was broken in 1.4.0.5.

@Phergus
Copy link
Contributor Author

Phergus commented Apr 28, 2019 •
edited
Loading

Also broken in 1.3.b87, 1.3.b89 and 1.3.b91. May have been broken forever.

@Azhrei
Copy link
Member

Azhrei commented Apr 29, 2019

@Phergus:

Second problem is that the Player Clients are getting Individual Views and not a shared PC view. With an owned token selected, player clients should get that token's individual view.

I don't understand. Did you mean, "player clients should NOT get that token's individual view"? Assuming you did...

First problem is that with Player View enabled on the GM client, no PC or NPC tokens are visible.

Yep, that's clearly a problem. Surprised this hasn't come up before, given that you found it's been like this since 1.3b87!

Related is that a player client with no token ownership does get the all PCs view.

Which makes sense to me, since there's no individual views.

The expected view for Player Clients is what is seen in MapTool with no server running and no connection to a server. With Player View enabled and no tokens selected, all tokens within the viewable area of any PC tokens are shown. With a single token selected, only those tokens within the selected token's viewable area are shown.

IIRC, when there is no server running, there is no concept of Player vs. GM, so the Player View option has nothing to work with.

It sounds like there might be a missing isPlayer() and/or isGM() in the code that determines which tokens to display. Digging through that code is likely to take some time. There are a lot of interactions between that code and other parts of MapTool (like the pointer tool rendering templates or the network layer tracking the movement of remote tokens)...

In any case, good catch on detecting the problem!

@JamzTheMan
Copy link
Member

Woot, I didn't break it this time lol

So honestly if it's been broken since b87 I probably won't touch it in favor of moving on to 2.0...

@Phergus
Copy link
Contributor Author

Phergus commented Apr 29, 2019

@Azhrei

Second problem is that the Player Clients are getting Individual Views and not a shared PC view. With an owned token selected, player clients should get that token's individual view.

I don't understand. Did you mean, "player clients should NOT get that token's individual view"? Assuming you did...

The server has Individual View turned off. The player client, with no tokens selected, should get the combined view of all Player tokens, owned or not. This would match the view in the Top-Right. Instead, with the existing code, the player logged in as Elf only sees what the Elf token sees. If MT was working correctly, and the Elf player selects an owned token, then they should get only the view of that token and not the combined view. Currently, that is the view the player gets whether the token is selected or not. See bottom-left and bottom-right. This is wrong.

Related is that a player client with no token ownership does get the all PCs view.

Which makes sense to me, since there's no individual views.

That is a contrasting statement showing that a client without any token ownership gets the correct view (Top-Right) while the client that actually owns a token on the map does not.

It would be best if you just fired up 2 or 3 MT clients and looked at it.

@Merudo
Copy link
Member

Merudo commented Aug 8, 2019 •
edited
Loading

The first issue (no vision when Player View enabled on the GM's client) is indeed a problem.

However, I'm having no trouble at all with the second issue. The players can see everything just fine.

EDIT: I figured it out, the bug doesn't happen when Strict Token Ownership is off.

Merudo added a commit to Merudo/maptool that referenced this issue Aug 8, 2019
@Merudo
Fix broken Player Vision for GM with Non Individual View
0fe9a59 
Previous code skipped all tokens if a GM had player view on, because of the incorrect code:

if (!isGMview && MapTool.getPlayer().isGM()){
   continue;
}

Fix the first issue of RPTools#357
@Merudo Merudo mentioned this issue Aug 8, 2019
Merged
@Merudo
Copy link
Member

Merudo commented Aug 8, 2019 •
edited
Loading

My commit fixes the first issue.

Merudo added a commit to Merudo/maptool that referenced this issue Aug 8, 2019
@Merudo
Fix broken Non Individual View for players
bfff141 
Fix getPlayerView() to return the correct set of tokens when no tokens are selected and the view is set to non individual.

This together with RPTools#550 fix RPTools#357.
@Merudo
Copy link
Member

Merudo commented Aug 8, 2019

My second commit fixes the second issue. Non Individual View should work fine now.

Merudo added a commit to Merudo/maptool that referenced this issue Aug 8, 2019
@Merudo
Fix broken Non Individual View for players
d60a30d 
Fix getPlayerView() to return the correct set of tokens when no tokens are selected and the view is set to non individual.

This together with RPTools#550 fix RPTools#357.
@Phergus Phergus added the in progress Issue is actively being worked on label Aug 8, 2019
Phergus added a commit that referenced this issue Aug 10, 2019
@Phergus
Merge pull request #550 from Merudo/PlayerVision
61ccfee 
Fix for broken Player View mode on GM client, and Player client view when Non-Individual View is chosen on server.  Issue #357
@Phergus
Copy link
Contributor Author

Phergus commented Aug 10, 2019 •
edited
Loading

Testing Server with Individual View unchecked:

  • GM Client - View as Player
  • GM Client - View as GM
  • Player Client - With no token ownership
  • Player Client - With single token ownership (Elf) - No token selected
  • Player Client - With single token ownership (Elf) - Owned token selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - No selected tokens
  • Player Client - With multiple token ownership (Mystic, Eagle) - Mystic selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - Eagle selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - Both selected

Testing Server with Individual View checked:

  • GM Client - View as Player
  • GM Client - View as GM
  • Player Client - With no token ownership
  • Player Client - With single token ownership (Elf) - No token selected
  • Player Client - With single token ownership (Elf) - Owned token selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - No selected tokens
  • Player Client - With multiple token ownership (Mystic, Eagle) - Mystic selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - Eagle selected
  • Player Client - With multiple token ownership (Mystic, Eagle) - Both selected

Testing No Server:

  • GM Client - View as Player
  • GM Client - View as GM

@Phergus
Copy link
Contributor Author

Phergus commented Aug 10, 2019

All seems to be working as expected.

@Phergus Phergus closed this as completed Aug 10, 2019
@Phergus Phergus added tested This issue has been QA tested by someone other than the developer. and removed in progress Issue is actively being worked on labels Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Merudo Merudo

Labels
bug high High priority bug/enhancement tested This issue has been QA tested by someone other than the developer.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Azhrei @Merudo @JamzTheMan @dorpond @Phergus