Foohy
commented
1 year ago I'm not sure what I was smoking at the time but that all looks great.
I think the only question I could ask here is, despite the comment stating as such, should it be calculating shard count by adding instead of multiplying? (undefined behavior excluded of course) Personally I don't think I've hit a big map and felt the shard count was too small (or hit a small map and felt it should've been more), and if suddenly a bunch of maps are just capped at 24 then it gets a bit less interesting there, as well as progression being generally tuned to get 13 shards maximum per map (beating the game in 5 maps is not intentional).
I'm going to merge it in anyway because wow possibly spawning 233,417 is not great (even if it didn't generally happen in practice), but something I'd definitely want to hear others take on.
I recently decided to look at the code for spawning Shards and discovered a bug that's been lying in plain sight since Commit https://github.com/Foohy/jazztronauts/commit/2894e6e4026ad5b9a446381ddb7d319a0667f1b8 from 2018. The area for a rectangle is width times height, but it was improperly calculated as width plus height. As a result, the area variable would be significantly less than the actual area of the map. The code defined the maximum number of Shards for a map as 24, but it was actually 13 in practice due to this miscalculation. If we assume a map is loaded that has the dimensions of 32768x32768 Hammer Units, which is the maximum that can fit within the Hammer Grid, the old code would return a value of 46,340 Hammer Units for its area. This would result in a calculated shardcount of 12.33 Shards, which is then rounded to 13 Shards. However, the map's actual area is 1,073,709,056 Hammer Units and it should get the maximum shardcount of 24 Shards.
I found a second latent bug that needed to be fixed as well, as giving math.Remap a value outside of the input range results in undefined behavior. Maps with areas smaller than 8,000 would spawn 2 or 3 Shards, instead of the defined 4 Shard minimum, and maps with areas bigger than 100,000 could spawn more than 24 Shards. For example, that maximum-sized map would attempt to spawn 233,417 Shards. I fixed this by clamping the returned area so that it would always fall within the expected range.
I additionally did some code cleanup: by removing a redundant call to math.max, as it would always return the number from math.Remap. by making the negative number handling in the area calculation use math.abs to get the positive absolute value instead of squaring and then square-rooting the numbers.