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

Detect script changes deeper in the extensions folder #3491

Open
juanitogan opened this issue Oct 5, 2022 · 5 comments
Open

Detect script changes deeper in the extensions folder #3491

juanitogan opened this issue Oct 5, 2022 · 5 comments
Labels
feature It's a feature, not a bug.

Comments

@juanitogan
Copy link

Is your feature request related to a problem? Please describe.

Changes to .mjs files in extensions\foo\modules, for example, are not detected and do not reinstantiate the script engine.

I have main.mjs in extensions\foo. It and its import modules in extensions\foo\modules load fine. However, only changes to main.mjs trigger a reload. Changes to any deeper module do not. If I move main.mjs down a level, changes to it never trigger (and everything actually unloads completely). If I move everything up a level to extensions\foo they all trigger a reload on change. But that's a bit messy.

Describe the solution you'd like

An extension can be placed either directly in an extensions directory, or in a sub-directory. All scripts files found in these directories are executed on startup.

Please detect scripts and script changes at least one level deeper. Or, at least do this when a .mjs file is found at the upper level.

@juanitogan juanitogan added the feature It's a feature, not a bug. label Oct 5, 2022
@bjorn
Copy link
Member

bjorn commented Feb 9, 2023

Hmm, indeed currently the extension folders are not watched recursively. Only the extensions folder itself, direct sub-folders and all directly loaded modules (so not modules loaded from modules) are watched for changes.

Maybe there is a way for Tiled to know which other files a script module loads, so it can watch it for changes. Or maybe it should indeed just watch the extensions folder recursively (potentially filtering on script files).

@firstmiddlelast
Copy link
Contributor

I'm having a similar issue here : files in extensions/tiled-script-main/Terrain Rectangle Tool/* are not detected nor loaded.
Also, I would suggest to not filter on script files by default, because changes in icons and other data files would not cause a refresh. In the end, I'd vote for simply watching the extensions folder recursively.

@eishiya
Copy link
Contributor

eishiya commented Apr 5, 2023

Also, I would suggest to not filter on script files by default, because changes in icons and other data files would not cause a refresh. In the end, I'd vote for simply watching the extensions folder recursively.

While it would be good to watch icons and such, I'd rather miss icon changes than have scripts reload every time a script writes a temp or config file xP
Ideally, I think Tiled should be able to detect images assigned as icons and watch those independently of watching scripts - changing an image used as an icon should change the icon to update, but reloading scripts should not be necessary. There are situations where changing an image used by a script should reload the whole script, but I think these are sufficiently uncommon/niche that it's reasonable to expect a user to reload scripts manually in that case (e.g. by touching/temp-modifying the relevant script file).

@cafeTechne
Copy link

If watched recursively, what is the base case?

@juanitogan
Copy link
Author

juanitogan commented Apr 5, 2023

I agree with @eishiya that it could get messy if not filtered. Personally, I have not felt much of a need for detecting icon changes even though I have changed the icon perhaps a dozen times by now. And, yet, purely from a technical point of view, I would have to agree that a RAD tool is not complete until all things that can be reasonably automated, have been automated... so it's a fair point to bring up. Regardless, I would file this as an optional stretch goal for whoever chooses to do the work, because the scripts are the primary focus of this RAD tool by at least 100:1. I would hate to see a feature delayed because of scope creep into nice-to-haves (that the Qt QML module may not be willing to provide easy hooks for).

Note that images have had a good RAD tool since the first paint program appeared. (But, yes, I have changed the icons at least once because they did not look good in the menus. But executing a manual reload for this minor use case is not oppressive either... unless I'm missing the use case.)

As for other supporting files... hmm, a bit tougher to get a read on that. I'm building a tool that uses dozens of PNG and TSX files, which are still evolving as well, and I have yet to feel changes in them should be picked up quicker, like how I feel about the code (or, maybe those ones are auto-reloading because of Tiled's TSX watchlist, and I just don't remember right now since my project has been on pause for several months [scheduled to resume in a week or so]). I was originally building with a fleet of TMX files too but dumped that idea quickly in favor of just coding all the automapping. In other words... Describe the use case for watching other supporting files so that we may understand it more.

BTW, I didn't mention in my OP that my current workaround to this issue is that I keep an empty dummy-reload.js file in the extensions folder. I also keep it open as the first file in my code editor. I've made a habit of resaving that file to trigger Tiled reloads whenever I'm done editing a script. Why I find this easier than reloading in Tiled, I forget (was it a command-line thing?), but I do. Other workaround hints are to not keep JS files deeper than extensions\foo and, if you need to organize your code more than that, learn to use MJS files instead of JS files with import statements for deeper MJS files.

Regardless of our responses, thanks for piping in and for your vote of support for some sort of solution here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature It's a feature, not a bug.
Projects
None yet
Development

No branches or pull requests

5 participants