ES6 related refactoring

[Allsimon]

Hello,

This PR introduces two minor changes:

• terminate every statements with a semi-colon (it was sometimes there, and sometimes omitted)
• use const and let if possible

I left some var as is in a few places as they might be bugs indicators.

For exemple, in lightmap.ts line 376:

        var v44, v4c, v38, v34, v28, v1c, v28;

The second v28 might be a failed copy pasta (benign) or a failed refactoring (dangerous)

[darkf]

Neat, been meaning to do this myself for a while (current approach has been to just use ES6 in code I touch at time time.)

One thing I notice in the diff is that the indentation changes/breaks, not sure if that's due to your editor or if the input source was just misusing tabs/spaces, but it looks mismatched.

Other than that (if it is a tab/space mismatch I guess I'd better run a tab->spaces formatter), if it does not change semantics it should be fine.

[Allsimon]

Well spotted, IntelliJ has hidden it nicely. Some files (char.ts) use four spaces, some use tabs (audio.ts) and some use both (vm.ts is really weird).

I can use an automatic reformatter, which style do you prefer ?

[darkf]

4 spaces for sure. Sorry about that, I've been meaning to make it consistent for a while now (historically I used Sublime as my editor which defaulted to tabs, but I usually set spaces, although it mis-interpreted them or I forgot sometimes, so it ended up with a mix of both... :P)

[Allsimon]

4 spaces everywhere ! :)

I wanted to enhance the combat system a bit, but I'm not really sure where to begins. I've seen some articles such as http://fallout.wikia.com/wiki/Fallout_combat or https://fallout.gamepedia.com/Fallout_and_Fallout_2_combat . Is there a better documentation somewhere ?

[darkf]

Depends what you're looking for, here are some formulas, there are damage calculations, etc. For anything more specific there's usually some resources found by googling, or in the worst case it can be RE'd or approximated.

Basic ranged/melee combat works AFAIK (minus critical effects, which are registered but don't do anything), throwing doesn't work yet (it's a bit silly), and probably some special weapons like the flamethrower don't work.

I'll review this later to make sure nothing changed semantically, thanks a bunch.

[Allsimon]

Thanks, I'm going to have to read a bunch of things before being useful :)

In the meantime, I added some really minor changes (use LF everywhere instead of CRLF).

[darkf]

So for this:

• The semicolon commit is fine, but it doesn't add semicolons to the last statements :P May as well be consistent
• The let/const commit changes formatting like info.numFrames-1 -> info.numFrames - 1 whereas I wrote the former because it's more compact and just as legible. Same for the let mapAreas: AreaMap | null = null; I think T|null is more compact/legible than T | null (although I wish ts had built-in syntax for nullable types like it does for undefined.) In general I'd recommend (for any project) that you cleanly split different formatting changs into multiple commits.
• The line endings don't really matter to me honestly, being CRLF is fine, at the very least it will prevent people from using the wrong core.autocrlf setting when cloning I guess :P
• I think the tsconfig is like that because it makes it easier to exclude files, and IIRC something screwy with file order. If you're sure it doesn't break anything and there's an easy way to exclude source files, this is fine though.

The whitespace change is good (didn't check all of it, assume you just ran a formatter over it.)

[Allsimon]

Same for the let mapAreas: AreaMap | null = null; I think T|null is more compact/legible than T | null (although I wish ts had built-in syntax for nullable types like it does for undefined.) In general I'd recommend (for any project) that you cleanly split different formatting changs into multiple commits.

Sorry, auto-formatter kicked in :/. I just removed them

info.numFrames-1 -> info.numFrames - 1

This rule is hard to enforce consistently. const sx = 4752 + (32 * y) - (48 * x); should be const sx = 4752+(32*y)-(48*x); ?

I think the tsconfig is like that because it makes it easier to exclude files, and IIRC something screwy with file order.

I thought you excluded files by removing them from the html files. I didn't notice any regressions and I think the typescript compiler would have thrown something if he couldn't compile.

[MaxDesiatov]

any updates on this? I was looking to contribute to darkfo, obviously more comfortable with ES6, so thinking whether it makes sense to fork from darkf/darkfo or Allsimon/darkfo?

[darkf]

Sorry for not getting around to this sooner, I was quite buy and then I kind of forgot about it!

So this is my plan for now:

• I like the tsconfig and gitignore changes
• I like the const/let changes assuming they don't touch anything else
• The other non-whitespace formatting stuff is a bit iffy, if you can post what formatter you're using perhaps I can look its settings over and give further advice (plus committing the config of such would be ideal anyway.) For example the x|y or x-1 stuff needs to be tweaked a bit.
• The semicolon change should be left off pending the former, where it will be done automatically (yours also doesn't terminate the last statement in a block which is a bit odd)
• The whitespace stuff is good, but please exclude the LUTs and colortest.html for now (the latter is only preserved for reference purposes)

Basically, keep the tabs->spaces, const/let, and tsconfig/gitignore, but please drop the extra formatting for the moment until I can find the right autoformatter settings

@maxdesiatov It already is ES6 with some ES2017 features (still migrating a lot of the code over to newer practices though), this is just formatting.

[darkf]

Haven't heard back on this so closing for now. I went ahead and converted the code over to using spaces, so that step is done at least. I don't quite like the way autoformatters are so rigid with being consistent in every single context...