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

More robust way of checking for VTTs #788

Open
jp-larose opened this issue May 3, 2021 · 2 comments
Open

More robust way of checking for VTTs #788

jp-larose opened this issue May 3, 2021 · 2 comments

Comments

@jp-larose
Copy link

Is your feature request related to a problem? Please describe.
I use Foundry VTT for my game. Beyond20 was working before and then stopped working. It's only today that I realized it's because I was using the "MyTab" module to change the favicon and title of the page. After digging in the source code here I discovered what Beyond20 was looking for in utils::isFVTT, it requires the title to have very specific text.

I'm sure checking the title covers > 90% of cases. However, there are situations, like mine, where the test yields a false negative. I can also foresee ways where it could yield a false positive (e.g. an page that's just an article with the right page title).

Describe the solution you'd like
Change the VTT check functions to look for variables or other context that's more specific and more reliable. E.g. for Foundry, check if window.game exists and (maybe?) game.system.id == 'dnd5e' (if that's the only system you're willing to support). I don't really know what the "right" check is, these are just ideas.

Also, I'm not sure how to do a similar check on the other VTTs you support, but it's probably possible for those platforms to have some way to muck up the title in a way that Beyond20 currently cannot recognize.

Describe alternatives you've considered
I looked at game variables in Foundry, none of it really screams "I'm a Foundry VTT Instance!" in a way that's reliable.

Another, simpler, option would be to document in the UI that Beyond20 is looking for specific strings in the page title to determine if it's a supported VTT platform and which one it is.

Additional context
In my case, when I used the "MyTab" module, it was primarily to change the favicon. In the process, I also changed the title and it included "Foundry", but not "Foundry Virtual Tabletop". Including the full string fixed the problem. But it also highlighted how fragile the check was.

@kakaroto
Copy link
Owner

kakaroto commented May 3, 2021

This used to be an issue, but I changed the algorithm in 2.4.0 (See #696) so that it checks for Foundry based on whether the foundry.js script file is part of the inclusions in the header, see the simple detection algorithm here : https://github.com/kakaroto/Beyond20/blob/master/src/fvtt/check-tab.js

You have a good point though about other parts of the code checking for whether it's foundry based on the title, but I think those should still function because Beyond20 itself will change the title after you activate the extension.

@kakaroto kakaroto added this to the 2.6 milestone Sep 10, 2021
@kakaroto
Copy link
Owner

So, something interesting here is that the check-tab.js doesn't seem to really get executed other than on Firefox. Otherwise, there are a few other places where the isFVTT check is done which are external places (background script, popup/settings menu) which are all places which do not have access to the actual DOM to do the check and the only thing they have access to is the title, so this would be difficult to separate the two...

@kakaroto kakaroto modified the milestones: 2.6, Maybe some day Dec 25, 2021
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

No branches or pull requests

2 participants