search

thisismypassport / shrinko8

Shrink (minify) Pico-8 carts, as well as other tools (e..g linting, format conversion)
MIT License
92 stars 7 forks source link

add globals: ls, dir, tostring, scoresub, _pausemenu #18

Closed pancelor closed 1 year ago

pancelor commented 1 year ago

tostring() works in pico8 (0.2.5g). It seems like an oversight but shrinko8 should not rename it; it's very easy to accidentally use tostring() instead of tostr(), which creates errors like "attempt to call global 'ol' (a nil value)" after running through shrinko8

after fixing this, I checked to see if there were any other globals: for k,v in all(_𝘦𝘯𝘷) do printh(k.." : "..tostr(v)) end

I added a few (ls, etc) but also found a bunch more; I'm not sure if these should be added or not:

flip : [function] trace : [function] __type : [function] _end_of_program : 1 _mark_cpu : [function] _menuitem : [function] _set_mainloop_exists : [function] _startframe : [function] _update_framerate : [function] backup : [function] bbsreq : [function] cd : [function] exit : [function] export : [function] folder : [function] help : [function] import : [function] info : [function] install_demos : [function] install_games : [function] keyconfig : [function] login : [function] logout : [function] mkdir : [function] radio : [function] reboot : [function] reset : [function] save : [function] shutdown : [function] splore : [function]

(most of them seem useless / nonfunctional from within a cart, but they also don't throw errors, and if shrinko8 renames them then they'll throw nil function errors... but they also don't seem to fit the names "deprecated_globals" or "undocumented_globals")

thisismypassport commented 1 year ago

I'll give this some thought, thanks.

pancelor commented 1 year ago

hmm, I've just realized that I use DIR as "direction" all the time (and I've seen it in others' code too) and it'd be a bit annoying for that to not be minified... but it is a sometimes-useful function -- I know people have built their own splore-likes using it. on the other hand, LS is mentioned in the manual (including runtime usage) while DIR is not...

I don't feel too strongly about most of this commit, but I do think the tostring part should be merged -- it caught me offguard a week ago, and a few days ago I was helping someone use shrinko8 for the first time and it gave them a very confusing error: "attempt to call global 'ol' (a nil value)" on a huge minified line of code, with no clear way to track down what on earth that meant, as a beginner. (this was before they had gotten shrinko8 to successfully shrink any carts)

thisismypassport commented 1 year ago

'ls' makes sense, 'tostring' I'm less sure about (it did seem to survive a good while, but I wouldn't be surprised if it goes away one version; plus I could see it being used for custom functions) but okay - merged, thanks. I think I'll also add an undocumented --not-builtin to allow to override this sort of thing. (There's already an undocumented --builtin) Ideally shrinko8 could try to analyze which builtins are definitely assigned before they're accessed and automatically treat them as custom globals, but that could run afoul of _ENV shenanigans...