Polishing map view

[maxpetul]

This PR makes various improvements to MapView that I've been thinking about for the past couple of months but hadn't gotten around to. Most importantly it significantly improves performance. On my system when fully zoomed out on our test map, these changes increase framerate by about 65%. Other than that it contains code cleanup, for example moving some functions to GameMap that belong there and revising some comments that have fallen out of date.

[WildWeazel]

quick tip, this can also be written as:
return this.tile.ExtraInfo.BaseTerrainFileID.CompareTo(other?.tile.ExtraInfo.BaseTerrainFileID);
(notice the ?.) since CompareTo(null) returns a positive value

[WildWeazel]

Great optimization! I'll let someone more familiar with drawing code review it.

[QuintillusCFC]

Looked through it in more detail, and it makes sense, including that it would speed things up. Left one question-comment about C# syntax meaning, but no need to wait for a re-approval before merging.

[QuintillusCFC]

Is the unusual braces syntax on lines 80-87 actually causing anything to happen, or is that just trying to say "this code is used to set the value of xInBounds"? I saw that the other day and saw that you wrote it in November and was wondering what was up with it. Usually if I wind up with braces in the middle of a function with no structure (if/for/try/etc.) around them, it's because I've made a mistake like accidentally deleting the line above them, so it made me wonder why those braces were there.

[maxpetul]

You're right, the braces are only there to contain the code that initializes xInBounds. I got into the habit of doing that after dabbling in Rust a few years ago. Rust is an expression based language, meaning everything is an expression and hence evaluates to some value. Semicolons chain expressions together kind of like the comma operator in C/++. Blocks of code inside braces also evaluate to a value, the result of the last expression in the block, and so do if "statements" (there are no statements in Rust, only expressions). So in Rust you can initialize variables very neatly by assigning to them the value of a block. For example, that xInBounds initialization would look something like this (maybe not exactly right, it's been a few years):

let xInBounds = {
    if wrapHorizontally {
        true
    } else if evenRow {
        (x >= 0) && (x <= numTilesWide - 2)
    } else {
        (x >= 1) && (x <= numTilesWide - 1)
    }
};

Notice there's only the one assignment. Actually in this case you wouldn't even need the outer braces, you could just assign the if expression to xInBounds directly.

Anyway, I like this b/c it makes it clear that the purpose of the block of code is to initialize a variable and it also means any helper variables declared inside that block won't clutter up the outer scope. So I started emulating this in C/++, although of course there you need to do the variable assignment manually inside the block so it's not as neat. In C# it makes even less sense since C# doesn't have block scoping, so I've generally been avoiding doing this, for that reason and also because it's unusual.

[QuintillusCFC]

Ah yes, I also dabbled in Rust a few years ago. I read about half of The Book, thought it was pretty neat, decided to port one of my old VB projects to Rust, realized I didn't understand the ownership concept as well as I thought when things weren't working, and a few days later Advent of Code started and I dove into that using C; so far I've yet to revisit Rust. But I did like that sort of syntax, particularly when written like you did in your comment.

It doesn't feel very idiomatic in C#, though; a small helper method would probably be the usual way to do things? As a one-off it's probably okay, but we probably generally want to stay idiomatic so newcomers who are familiar with C# recognize the constructs.

[maxpetul]

Cool, I didn't know you were interested in Rust. It's an interesting language with a lot of good ideas. It's kind of a shame the borrow checker is such a prominent feature because it makes it difficult to write "proper" Rust. My experience with the language is similar to yours, I learned half of it and built some toy projects along the way, but I never felt like doing anything substantial with it because I didn't want to fight with the borrow checker. I totally understand people who say the best way to use Rust is to wrap the whole program in unsafe{...} and think of the language as a better C++. Using Rust for C7 might have been fun but probably not practical, and anyway it's too late to second guess the language now.