Bolt-Scripts / UI_Scaler

2 stars 3 forks source link

Proper way of using lua modules. #4

Closed Diyagi closed 2 years ago

Diyagi commented 2 years ago

So, i noticied that your module was actually using the global namespace, that is not how a module is supposed to work (as far as i know, im not a lua master, so, if im actually wrong, i would love to know !).

I fixed it and moved it to a folder so reframework dont run the module alone (without it being imported).

Plus, i added the reframework`s globals into the cfg so the lua extension for VSCode dont treat them as undefined variables.

Bolt-Scripts commented 2 years ago

I didn't even know the require thing worked like that. Makes sense I guess but lua just kinda sucks. Anyway I doubt it would've caused many issues but probably a good thing to change anyway. Though I'm pretty reluctant to move that file into another folder because trying to communicate that people should remove the old file would just be annoying, though technically it probably won't hurt anything much for it to exist. idk.

Diyagi commented 2 years ago

I didn't even know the require thing worked like that. Makes sense I guess but lua just kinda sucks. Anyway I doubt it would've caused many issues but probably a good thing to change anyway. Though I'm pretty reluctant to move that file into another folder because trying to communicate that people should remove the old file would just be annoying, though technically it probably won't hurt anything much for it to exist. idk.

You dont have to move actually, praydog already told that, it should not give any problems. If you want, i can remove this commit.

You can actually, compile everything in a single file, to be more user friendly, but still maintaining a good code so other developers can understand what thing does and help if they want.

praydog`s advice

Bolt-Scripts commented 2 years ago

I just wanted to split it up a bit for my own sanity, but this whole thing is even more annoying than I thought it was lmao. I'll just go ahead and use your changes, it's scuffed either way ¯_(ツ)_/¯

Bolt-Scripts commented 2 years ago

Not sure if you tested this before posting but I did have to do some fixes to make it actually run. It didn't like that the elementDatas was a table but also trying to be used as a module.

Bolt-Scripts commented 2 years ago

Also doing this incidentally fixes a bug that was preventing stuff from loading properly after the last refactor i did which makes absolutely no sense but just add it to the stack of reasons why lua sucks. But thanks I guess. That would've driven me insane trying to fix but also it still kinda does because I don't get how this fixes it anyway. I assume it's just some absolutely arcane scope based nonsense, but sure.

Diyagi commented 2 years ago

Also doing this incidentally fixes a bug that was preventing stuff from loading properly after the last refactor i did which makes absolutely no sense but just add it to the stack of reasons why lua sucks. But thanks I guess. That would've driven me insane trying to fix but also it still kinda does because I don't get how this fixes it anyway. I assume it's just some absolutely arcane scope based nonsense, but sure.

Well, thats that is why i dont like global namespace, strange things happens.

I actually did test it, but im changing more things in my local repo, so i might have fixed it and didnt noticed the problem was in the PR too, sorry about this.

I might open another PR later.