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

Reword compatibility warnings and improve README to be more user-friendly #77

Closed
manuelVo opened this issue Jun 21, 2023 · 4 comments
Closed
Labels
enhancement New feature or request

Comments

@manuelVo
Copy link

manuelVo commented Jun 21, 2023

Whenever two modules hook the same function, libwrapper emits a warning to the user that the two modules are potentially incompatible. There is a user base of foundry, that doesn't read this warning as what it is - a hint to check if the modules work together properly - but rather as an error message that informs them that there are in fact problems. Some of these users then go to the repositories of the respective modules and open an issue reporting a module compatibility. Those bug reports usually end up being false positives, so every time a user opens an issue like this the author of that module wastes some of their module development time by investigating an issue that doesn't exist.

Additionally, I suspect that for every user that's getting in touch with the community to ask about that error, there are probably 10 users who will never reach out, but who will uninstall one of the two modules to avoid breakage, not knowing that the modules are fully compatible.

To solve this, I'd like to ask for the warning message to be reworded in a way that makes the fact that there might not be any issues after all more obvious to the users, so that they are less likely to go a report a bug before they actually encounter one.

To make this issue as constructive as possible, here's a suggestion for how a clearer warning message could look like:

The modules Foo and Bar modify the same functionality of FoundryVTT.
Modules that do this have an increased likelihood of conflicting with each other.
@manuelVo manuelVo added the enhancement New feature or request label Jun 21, 2023
@ruipin
Copy link
Owner

ruipin commented Jun 21, 2023

Thanks for the suggestion. I agree that the message could be improved, however there is a limited amount of space for the message. Unfortunately, your suggestion is a bit too long as it wouldn't fit on screen without using multiple lines.

Changing the existing message is easy, and it can be found here:
https://github.com/ruipin/fvtt-lib-wrapper/blob/master/lang/en.json#L128

Potential conflict detected between {main} and {other}.

I'd prefer if the new message has a comparable size, so it could fit in a single line.

Do you have any ideas for a message that would be shorter? For example, what do you think of:

{main} and {other} modify the same FoundryVTT functionality, and therefore are more likely to conflict.

@manuelVo
Copy link
Author

I like your suggestion. In fact I like it much better than what I came up. Your message is quite short, which increases the chance of users actually reading it while while still maintaining a nice balance between informing the user about potential conflicts without being worded too definitively.

@ruipin
Copy link
Owner

ruipin commented Jul 18, 2023

Sorry, I've been quite busy lately. Planning to tackle this soon, hopefully this week.

In addition, someone suggested I update the intro to the Readme to make it easier to understand why libWrapper is useful, specifically:

  • Able to detect multiple types of conflicts, and show them to the user.

  • Encourages a standard Monkey-patching API, rather than every module developer doing their own thing. This improves consistency and reliability, and reduces the likelihood of mistakes creating unnecessary conflicts.

  • Controls the wrapping order, rather than leaving it to the order modules are executed. This decreases the chance of conflicts, since modules can specify their requirements - do they need to run first, last, or maybe they don't really care?

  • LibWrapper is able to handle various edge cases which can be complex to resolve otherwise, such as wrapping getters and setters.

  • Includes advanced error detection, which simplifies troubleshooting.

    • When an error occurs inside a wrapper, libWrapper will be able to point out exactly which package caused it.
    • When an error occurs outside of a wrapper, libWrapper will attempt to discover which packages (if any) might have caused it.

These need a bit of proofreading/clean up, but should probably be mentioned at the start of the readme.

@ruipin ruipin changed the title Reword the warning about potential conflicts to make it clearer to users that the modules might be fully compatible Reword compatibility warnings and improve README to be clearer about why libWrapper is useful Jul 18, 2023
@ruipin ruipin changed the title Reword compatibility warnings and improve README to be clearer about why libWrapper is useful Reword compatibility warnings and improve README to be more user-friendly Jul 18, 2023
@ruipin
Copy link
Owner

ruipin commented Jul 18, 2023

Resolved in v1.12.13.0. Closing.

@ruipin ruipin closed this as completed Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants