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

[Bug]: Vision broken for imported UVTT maps on player client side #3434

Closed
Phergus opened this issue Jun 4, 2022 · 9 comments
Closed

[Bug]: Vision broken for imported UVTT maps on player client side #3434

Phergus opened this issue Jun 4, 2022 · 9 comments
Assignees
Labels
bug tested This issue has been QA tested by someone other than the developer.

Comments

@Phergus
Copy link
Contributor

Phergus commented Jun 4, 2022

Describe the Bug

Vision on the player clients is broken on imported UVTT (i.e. Dungeondraft) maps. Symptoms are that the player view does not get the vision outline and may not be able to see other tokens. Exposing visible area on maps with FoW on the player client will expose the entire map. On the GM client vision and exposing work correctly.

In some cases, erasing VBL on doors and/or redrawing over the existing VBL will allow vision to work correctly on the player side.

Saving out the campaign and then reloading the campaign appears to fix the issue.

To Reproduce

  1. Import some UVTT map files (see attached) and set Lights to Day on the import dialog.
  2. Make sure maps are visible to players.
  3. Start Server.
  4. Connect with player client.
  5. On player client drag one of the Default tokens onto a map within an room or other enclosed space.
  6. Ensure that token has Normal vision (or equivalent).
  7. Observe that hovering the mouse over the token does not show the visible area outline.
  8. On server client, hover over token. Visible area outline will be visible.
  9. Enable FoW on server for current map.
  10. Switch back to player client and using Map Explorer right-click on token to expose. Entire map will be exposed.

The problem is not consistent. Some maps have worked one time and not the next.

As mentioned above, saving the campaign and reloading it appears to correct the issue.

Expected Behaviour

Expect vision and exposing to work correctly on player clients.

Screenshots

No response

MapTool Info

MT v1.10.4, 1.11.5 and current dev branch

Desktop

Windows 10

Additional Context

Zip file contains 4 different UVTT test maps.

Dungeondraft UVTT files.zip

@Phergus Phergus added the bug label Jun 4, 2022
@taustinoc
Copy link

When I was experimenting with the campaign file on Discord, I noticed that the vision outline also didn't work on the server. (Seeing the token wasn't an issue.)

@kwvanderlinde
Copy link
Collaborator

I haven't been able to reproduce this using the given steps (maybe once, but I'm not sure I actually did it the right way). But I have found a consistent way to get the same symptoms, just by doing things in a different order:

  1. Start server
  2. Import UVTT maps
  3. Connect with player client
  4. etc...

Also, I haven't played with FoW yet, this is just observing the vision outlines.

Here's what I have gleaned so far (I haven't had much chance to look into it yet):

  • When starting a server, the campaign (including all zones) is copied. The server itself has direct access to the original, while the server API deals with the copy.
  • When a zone is added to a campaign after the server is started, the zone is added directly to the server's campaign instance, while the campaign copy (and client's "copies" of the campaign) are updated via a server message. This results in a copy of the zone being stored in the copy of the campaign.
  • The DungeonDraftImporter creates a zone and adds it to the campaign (and a copy of the zone to the copy of the campaign, as stated above). After that, it adds VBL/MBL to its zone directly. The VBL/MBL is not reflected to the copy of the zone in the campaign copy, nor is it reflected to clients.

My hunch right now is that DungeonDraftImporter is to blame here because it directly adds to the zone without a corresopnding server command. Compare with TokenVBL.renderTopology() - it calls zone.addTopology(...) then MapTool.serverCommand().addTopology(...)

Of course, this hunch applies to my reproduction of the issue. I'm not sure yet how the steps given by Phergus would show this particular problem, except that there may be another code path that is capable of leaving the VBL/MBL blank after a zone copy.

@kwvanderlinde
Copy link
Collaborator

Further testing shows the same problem for tokens. E.g., the light tokens added by DungeonDraftImporter won't be synced to clients. The solution here is probably to either add the zone to the campaign only at the end of the import, or else avoid directly updating the zone and instead issue server commands.

@thelsing
Copy link
Collaborator

thelsing commented Jun 8, 2022

Maybe we should only enable the importer on a personal server?

@Phergus
Copy link
Contributor Author

Phergus commented Jun 9, 2022

On imported UVTT maps that don't have Lights the problem either doesn't happen or is very hard to reproduce. Not sure which it is as I was able to get the problem on all 4 of the attached maps but only when all 4 had already been loaded which includes the one with lights and it was the first map that I tried placing a token on.

There were no issues with these maps under MT 1.9.3. The VBL for walls and doors plus the light tokens all worked as expected and still do in 1.9.3.

After merging in the changes from #3437 vision appears to be working as before. Will test with some more maps later.

I'm not sure if campaigns being copied at server start directly pertains to this issue but it does seem like doing the MapTool.addZone() in the middle of the UVTT import, before the various topology pieces and light tokens are added to the zone, could be a potential issue.

Maybe we should only enable the importer on a personal server?

No.

@kwvanderlinde
Copy link
Collaborator

The fix in #3437 may incidentally fix the issue here as token state should be more consistent now (particularly for lights UVTT lights). It would mean that there is some other code path susceptible to these null token fields, but I think it should have been covered by accessor methods until now (can't check right now whether that is actually true). It was only protobuf that didn't handle the nulls because it read directly from the field.

If that is the case, then what I reproduced is, strictly speaking, a distinct bug because it applies just as much before and after that fix.

You're right that the campaign copy doesn't directly relate to this issue, but it is the explanation for why the server has correct vision while clients do not. The server has direct access the original campaign while the campaign copy (which is accessed via server commands) is what is consistent with player clients.

@kwvanderlinde
Copy link
Collaborator

Based on @Phergus' statement that 1.9.3 is unaffected by this bug, it's clear now that the issue I discovered is a deficiency in the UVTT importer itself whereas this issue is caused by something else (whether that is a partially initialized Token or something else). I've opened #3440 to track the issue with the importer.

Hopefully this issue really is just a result of some bad token state, and that #3437 can fix it. Unfortunately I still have not been able to reproduce this issue so I can't really dig any deeper into it.

@Phergus
Copy link
Contributor Author

Phergus commented Jun 11, 2022

Yeah. I think the issue here is just the fallout from the change to protobuf which exposed the lack of initialization of various token properties. Not properly updating client maps with VBL is different.

@Phergus Phergus added the tested This issue has been QA tested by someone other than the developer. label Jun 11, 2022
@Phergus
Copy link
Contributor Author

Phergus commented Jun 11, 2022

Tested various UVTT maps that produce light tokens. All working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tested This issue has been QA tested by someone other than the developer.
Projects
None yet
Development

No branches or pull requests

4 participants