GDColon / GDBrowser

A website that lets you browse all of Geometry Dash's online features, and more!
https://gdbrowser.com
MIT License
333 stars 161 forks source link

A lot of refactoring, and etc #232

Open Rudxain opened 2 years ago

Rudxain commented 2 years ago

Fixes #246 The title is self-descriptive. I also renamed misleading names of functions, and activated JS strict mode.

I made small style changes, like making 1-liner functions actually use 1 line, and splitting long lines of property access into multiple lines for easier reading

Rudxain commented 2 years ago

Wait, I forgot to check if any renamed function is used outside the file. I'll convert to draft

Rudxain commented 2 years ago

It seems no functionality should break after renaming, AFAIK

Rudxain commented 2 years ago

I need to do more refactoring and solve the commit incompatibility. Converting to draft

GDColon commented 2 years ago

(whoops sorry) new conflicts aside this looks good, i was just reluctant at first since theres things i do and dont like

Rudxain commented 2 years ago

It's okay. I realized I made a mistake in the formatting. For example, I found an else clause formatted like this:

if (...) {
    ...
}

else {
    ...
}

And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol

GDColon commented 2 years ago

It's okay. I realized I made a mistake in the formatting. For example, I found an else clause formatted like this:

if (...) {
    ...
}

else {
    ...
}

And I thought it was a whitespace typo because it looked weird and unique, but then I found it's actually a pattern found in many places lol

oh this is always how i've done it, pls dont change

Rudxain commented 2 years ago

lmao, I was using a VScode extension to edit repos remotely without cloning, and I thought I had to type the branch name everytime I wanted to commit. Sorry for any confusion to everyone. Now the commits are named using unhelpful names

Rudxain commented 2 years ago

I'm almost done with this PR. If I have enough time, it'll be ready in some hours (or a day) and the commit will also fix #246 .

I did some tests, but I didn't do any tests for stuff that I read enough (both GDB code and MDN docs). I also didn't test if activating strict mode throws an error somewhere lol, the code seems hi-quality enough that we don't need to worry about invalid code.

Also, randRange may not be available due to a race condition while loading global.js, but since the script explicitly starts loading in the HTML before randRange is called, it seems the glitch is unlikely to happen. I guess I can test that, because I don't need NodeJS for that (I don't have NodeJS installed yet, lol)

Rudxain commented 2 years ago

@GDColon It's ready. But I'm afraid there may be a bug or syntax error somewhere lol, sorry

Rudxain commented 2 years ago

Wait, I realized that the split method has a 2nd arg that can improve speed and memory use (only in engines that don't optimize well). Also, the XOR class can be refactored to convert all its prototype.* methods into static ones, that will allow you to do this:

XOR.encrypt(str, key)

instead of this:

const xor = new XOR()
xor.encrypt(str, key)

Edit: I found a possible problem, app.xor. It seems like that property must be an object instance of XOR, I don't know why lol

Rudxain commented 2 years ago

I need help refactoring obj.foo && obj.foo.bar into obj.foo?.bar. I don't know which ones can be replaced and which not, I tried searching but only got a vague idea

Rudxain commented 2 years ago

@GDColon I found broken <script src="..."> URLs, like "../icon.js" instead of "/iconkit/icon.js", and "../global.js" instead of "/misc/global.js". I'm fixing it now

Wait holup. I realized the deployed website has a different dir tree, so those paths seem to work?

I noticed iconkit.html doesn't run global.js, why is that?

Rudxain commented 2 years ago

I think this PR is too big now. I'll re-review to see if I made more mistakes, and only fix those mistakes in this PR. Any further changes will be done in a separate PR

Rudxain commented 2 years ago

I'm reverting those URLs because I didn't test if they work