danneu / elm-hex-grid

a hex-grid library for elm
17 stars 5 forks source link

Bring up-to-date for Elm 0.18. #2

Closed seagreen closed 7 years ago

seagreen commented 7 years ago

🚨 First ever Elm code I've written. You might want to look it over carefully=) 🚨

I tried to make changes following the Upgrading to 0.18 guide exactly. I did run into two issues where the compiler would no longer allow variables to be Int that weren't explained by the 0.18 changes, I noted both in comments.

danneu commented 7 years ago

Thanks, I'll check it out.

danneu commented 7 years ago

Hey, I had to make a lot more changes to get it 0.18-ready.

I was going to merge your changes and then do the rest but I totally forgot. Sorry.

Diff: https://github.com/danneu/elm-hex-grid/commit/558fa7ecc069b9ed0b663206dfcbef3d20926e52

I also broke the demo into individual models to improve the performance.

danneu commented 7 years ago

@seagreen For some reason I thought I was looking at the Files Changed tab and thought the 3 lines you commented on were the only lines you changed. You can tell I'm not used to getting contributors on Github. :rofl:

Thanks for the contribution!

seagreen commented 7 years ago

No worries, I dashed it off really quick anyway. Did you ever figure out what required the addition of the toFloat calls between 0.17 and 0.18?

danneu commented 7 years ago

@seagreen at a glance, it seems like Elm just fixed a bug.

@@ -498,9 +500,9 @@ pointRound (x, z) =
   let
     y = -x - z
     (rx, ry, rz) = (round x, round y, round z)
-    dx = abs (rx - x)
-    dy = abs (ry - y)
-    dz = abs (rz - z)
+    dx = abs (toFloat rx - x)
+    dy = abs (toFloat ry - y)
+    dz = abs (toFloat rz - z)

so it makes that rx - x i.e. int - float must be lifted back into float - float.