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

OverlayPlugin WS server usability improvements #165

Open
quisquous opened this issue Sep 7, 2020 · 3 comments
Open

OverlayPlugin WS server usability improvements #165

quisquous opened this issue Sep 7, 2020 · 3 comments

Comments

@quisquous
Copy link

quisquous commented Sep 7, 2020

A few thoughts when updating my documentation for this:

  • can ngrok-x64.exe be started hidden? There's already a start and stop button in the OverlayPlugin UI for this
  • closing the ngrok-x64.exe command window doesn't update the UI to say it is stopped
  • when restarting shared overlays (or switching tabs from shared to stream/local), the URL generator url doesn't update to reflect the new id
  • the URL generator should ideally use file:// urls for local files, as copying and pasting a local filename with a url query string escapes the question mark, e.g. C:\Users\tinipoutini\cactbot\ui\raidboss\raidboss.html?OVERLAY_WS=wss://75ee58f97e2e.ngrok.io/ws becomes file:///C:/Users/tinipoutini/cactbot/ui/raidboss/raidboss.html%3FOVERLAY_WS=wss://75ee58f97e2e.ngrok.io/ws in the url bar (this is clearly not something most people will do, but this is also true for stream/local overlays as well as shared, where it's more relevant)
  • closing ACT should kill any open ngrok process so it doesn't have to be closed manually before starting it again
  • "It works!" page should include ngrok query strings instead of local query strings if loaded over ngrok
  • cactbot presets in the url generator all point to local files, but these need to be the gh-pages version to be loaded by others. Is there a way to add a presets for stream/local vs shared? Or is the future that everything is remote so I should just make all cactbot presets remote now?
  • target bar urls in the url generator appear busted and look like %/%/targetbars/etc. (Although, arguably these should not be included in the preset list for shared overlays, as we don't know the target for the remote user. Similarly, cactbot jobs.)
  • If you know what you're using an overlay that isn't listed, here are some query strings for you: should probably say If you know what you're doing and you're using an overlay that isn't listed...
  • no feedback when the stream/local overlay can't open a port because ngrok is blocking it. You can repro this by: shared overlay -> start -> close ACT -> start ACT -> stream/local overlay -> start -> hang until ngrok exe is closed. It's worse that stream/local overlay says that it has started correctly so there is no indication something is wrong here.
  • similarly, no feedback the other way around for ngrok not working because stream/local overlay is blocking it. You can repro this by: stream/local overlay -> start -> shared overlay -> start. ngrok says it starts correctly and stream/local overlay transitions to "failed", however stream/local overlay is the only one working and shared overlay does not work. I can't fix this state without restarting ACT and closing ngrok.

(Sorry for the giant list. I can open separate bugs if these are things you want to fix, but this just seemed cleaner to put all of the shared overlay/ngrok issues in one place.)

@ngld
Copy link
Owner

ngld commented Sep 8, 2020

I went for some low-hanging fruit in 73ae931:

  • can ngrok-x64.exe be started hidden? There's already a start and stop button in the OverlayPlugin UI for this
    I think I made the ngrok window visible during testing and forgot to turn that option off once I was done.
  • closing the ngrok-x64.exe command window doesn't update the UI to say it is stopped
    This should work now but probably needs some polish (it just sets the tunnel status to Failed right now).
  • when restarting shared overlays (or switching tabs from shared to stream/local), the URL generator url doesn't update to reflect the new id
    Restarting shared overlays now updates the generated URLs. Switching tabs doesn't, at least not yet.
  • closing ACT should kill any open ngrok process so it doesn't have to be closed manually before starting it again
    This should work now.
  • "It works!" page should include ngrok query strings instead of local query strings if loaded over ngrok
    This is going to be difficult. Right now I generate the URLs on the server because I don't want to implement the URL generator twice (once in JS and once in C#). However, the server doesn't know if you're opening the page through ngrok or not since ngrok sends the same Host: header that a local browser would send for http://127.0.0.1:10501. I can change this but that requires a rewrite of the WebSocket implementation first. I want to get rid of WebSocket-Sharp (the current implementation) due to some annoying issues but it's fairly low priority for now.
  • Regarding the file:/// URLs: At least for the builtin overlays (target bars, enmity, etc.), OverlayPlugin already generates working file:/// URLs. If you're seeing %% characters, then the replacement fails somewhere (the preset URL uses %%/ as the placeholder for the local file path).
    The URLs are listed because they still work in OBS which is a common enough use for them as far as I'm aware.
    I can probably extend the URL generator to generate different URLs for overlays vs. WebSocket URLs.
  • The text typo has been fixed.
  • I should add some error messages if one of the two ports is blocked. Will look more into this tomorrow.

@quisquous
Copy link
Author

Thank you for all the fixes!

Yeah agreed that the "It works!" page is probably low priority. It was just weird to see.

If I have some time this week I'll look at adding different kinds of preset options (whether it supports overlay/shared/local), since I wanted to hide raidboss emulator and similarly my oopsy summary review in the overlay presets, and I think that could be similarly leveraged for turning off target bars, enmity, and some cactbot overlays for the remote presets.

Re: file:// urls. I guess there's two issues here.

  1. Are you not seeing that %% character for target bars and enmity? I'm using a fetched APPDATA version of OverlayPlugin, so I don't feel like there's anything special in my setup. I was pretty sure I tested this when I added enmity presets at least, but maybe I didn't properly?

  2. I'm not sure what you mean by "already generates working file:// urls. I guess I'm saying that for things like Ember or Kagerou, I see the https scheme. However, for cactbot, I see no file:// scheme at all and just the raw filename, which seems incorrect. For enmity and targetbars, there is likewise no scheme and also it starts with %%.

@ngld
Copy link
Owner

ngld commented Sep 9, 2020

If I have some time this week I'll look at adding different kinds of preset options (whether it supports overlay/shared/local), since I wanted to hide raidboss emulator and similarly my oopsy summary review in the overlay presets, and I think that could be similarly leveraged for turning off target bars, enmity, and some cactbot overlays for the remote presets.

Please don't remove target bars and enmity from the URL presets. At least leave them for local mode. I've heard from a few people who use an XL plugin that can display overlays in fullscreen mode (since XL plugins run inside the game process, they can hook the render loop). This obviously needs WSServer and at least one person uses the targetbar overlay with that setup.

  1. Are you not seeing that %% character for target bars and enmity? I'm using a fetched APPDATA version of OverlayPlugin, so I don't feel like there's anything special in my setup. I was pretty sure I tested this when I added enmity presets at least, but maybe I didn't properly?

I don't see the %% character. However, I forgot that I fixed that yesterday which means that it's not part of an official release, yet.

#if DEBUG
var resourcesPath = "file:///" + _plugin.PluginDirectory.Replace('\\', '/') + "/libs/resources";
#else
var resourcesPath = "file:///" + _plugin.PluginDirectory.Replace('\\', '/') + "/resources";
#endif
var url = preset.Url.Replace("\\", "/").Replace("%%", resourcesPath);

  1. I'm not sure what you mean by "already generates working file:// urls. I guess I'm saying that for things like Ember or Kagerou, I see the https scheme. However, for cactbot, I see no file:// scheme at all and just the raw filename, which seems incorrect. For enmity and targetbars, there is likewise no scheme and also it starts with %%.

If you want to see the file:// scheme for Cactbot files, you'll have to modify your presets in Cactbot. OverlayPlugin uses file:// URLs for all local files (see above). The URL generator uses the same URLs the overlay loads. If the preset doesn't contain the file:// scheme, the generated URL won't have one.

quisquous added a commit to quisquous/cactbot that referenced this issue Sep 12, 2020
sandtechnology pushed a commit to sandtechnology/OverlayPlugin that referenced this issue May 7, 2023
* Split MiniParseEventSource into multiple event sources

* Fix missing changes due to MSVS not saving the file
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