League-of-Foundry-Developers / fvtt-module-theatre

GNU General Public License v3.0
30 stars 39 forks source link

BIg rewrite in preparation for some new features #160

Closed p4535992 closed 6 months ago

p4535992 commented 8 months ago

I have been planning for quite some time now to integrate the https://github.com/cswendrowski/FoundryVTT-ViNo project into theatre to take the best of the two modules.

In reviewing the code I realized that I had dirtied the code quite a bit and the API was unobtainable for newbies (the usually search for game.modules.get("theatre").api.

I rewrote some code in preparation for the vino merge with what I consider to be the standard foundryvtt project structure (settings.js, constants.js, api.js, socket,js, moduel.js, ecc.).

I am asking other developers who have dedicated themselves to the project to give me their feedback to make the project as smooth as possible.

@mclemente @farling42 @megahead11 @JulieWinchester

Here is the module.json for the installation to try before accept the PR:

https://github.com/p4535992/fvtt-module-theatre/releases/download/2.9.1/module.json

farling42 commented 8 months ago

After doing some work with Theatre, I have stepped back and don't have it installed any longer.

megahead11 commented 8 months ago

Merging with vino seems like quite an endeavor. I know Vino's been kinda dead for a long time, especially since it was kinda posed as a replacement for Theatre (that didn't happen since it basically did none of what theatre did). That being said, I'm curious what parts exactly you wanted out of Vino in Theatre?

BrotherSharper commented 8 months ago

Heya, thanks for working on maintaining Theatre Inserts!

Theatre and Vino have different philosophies; Theatre is focused on the user being able to show and control the portrait independently, while Vino is used for automatically showing. Hence, they were split (I was there when talking to both authors).

Please ensure that current functionalities are still available since it would cause many problems for current playstyles.

mclemente commented 8 months ago

I don't use Theatre Inserts at all, honestly. I've once helped update it from V9 to V10 and then my other contributions were to fix some issues with Polyglot.

If you manage to make this maintainable, I'm all for it, specially if you take over the package to avoid issues caused by the League being mostly busy or inactive.

My major issue with Theatre Inserts is the way it is currently displayed. Having to always go "Alt + Enter" and "Shift + Enter" on reload is an awful experience, and the Nav Bar is super small. It barely fits 5 characters before you have to scroll. It feels like it should be larger and could use a world setting to keep it over reloads/sessions, but that's probably the least of the module's problems, all things considered.

JulieWinchester commented 7 months ago

Maybe bucking the trend of some of the other commenters here, I do still use Theatre Inserts, and it's pretty important for my games. But even still, I'm pretty much always on the edge of getting rid of it due to its various annoyances, and I only started making contributions to fix relatively minor things that really got under my skin.

It would be great to have someone really dig in and rewrite the entire module, perhaps getting rid of how it maintains an entire separate PixiJS canvas which is super difficult to work with. Philosophically, though, I'm not sure if I think it makes sense to merge Theatre with ViNo? As was mentioned earlier in this thread, the two modules seem to have pretty different approaches to how to handle showing NPC portraits, with ViNo having it happen automatically (and only on chat? if so, that limits people who don't use chat but want portraits) vs Theatre using staging. I sort of wonder if such a radical departure that merges the two might not be better as its own successor module?

Also, since some other general ideas have been raised, I agree with @mclemente that my major issue with Theatre right now is how the NavBar handles, and how it's kind of jammed inelegantly into the chat sidebar. Around Thanksgiving I was messing around with a NavBar rewrite that optionally pulls the NavBar out of the chat UI and makes it a separate UI window that can be called from a left toolbar button, makes it resizable, can add selected tokens to navbar with a button, etc. It wasn't too far off from being in a PR-able state, but I haven't had the time/motivation to finish it off since then.

p4535992 commented 6 months ago

I'll close this PR because I'm messed up and can't devote the time I wanted to this module, thank you all for the comments