search

mclemente / fvtt-advanced-macros

Use handlebars templating, recursive macro calls and call macros with arguments or directly from chat.
MIT License
15 stars 9 forks source link

Update the project with some feature and replaced the glp with vite and commonjs #84

Closed p4535992 closed 9 months ago

p4535992 commented 9 months ago

I tried to improve the form to make it uglier perhaps code-wise, but more usable by people who know little javascript.

Renamed main.js to module.js to follow typonjs plugin guidelines Updated github action when a release is created Replaced gulp with vite and commonjs Add check box "Run as GM" for better clarity (https://github.com/mclemente/fvtt-advanced-macros/issues/79) Add check box "Run as World script" for better clarity Add check box for "Run as a personal macro", to help player to run macro on the referenced actor Add some style to the form for better clarity Update module.jon with conflicts and recommended modules

mclemente commented 9 months ago

Hey, I appreciate your contribution, but you're doing a lot of things all at once.

Replaced gulp with vite and commonjs

I'm used to gulp on all my other projects. What are its advantages over gulp? Also, I think it would be better to just replace the current gulp for "ghost's Gulp + Rollup Preset" instead.

Add check box "Run as GM" for better clarity (https://github.com/mclemente/fvtt-advanced-macros/issues/79) Add check box "Run as World script" for better clarity

I don't think this is necessary, it bloats the form when the select already gets the job done.

The select options are mutually exclusive, having additional checkboxes that will ignore the select isn't doing "clarity" any favors. Do you really want to walkthrough people over why their macro that is set for a specific user and marked as "Run as GM/World Script" isn't being run as the selected user?

I don't even think the World Script feature is used that much considering World Scripter is always the recommended module for that feature.

Also, about the "Myself" option, why do they need that option when their name is listed right below?

Add check box for "Run as a personal macro", to help player to run macro on the referenced actor

Users shouldn't write macros differently from core macro logic, that will be a mess to get help writing macros and will be a pain to maintain over Foundry versions.

Also, that description shouldn't be displayed on the form, and shouldn't be assuming the user knows what Item Macro and Item Piles modules do.

Add some style to the form for better clarity

I don't see that anywhere.

Update module.jon with conflicts and recommended modules

Improved Macro Editor and Sidebar Macros haven't been updated in over a year now. They're not even verified for V11, they shouldn't be recommended.

Socket Macros doesn't even exist anymore, it isn't even displayed on theripper93's site.

p4535992 commented 9 months ago

i close the pr has invalid.