Fix hashing overflow

[liamdon]

Hello! Thank you for a fantastic and well-architected library, I love the approach.

Though I think anyone using this library is likely targeting 64-bit devices only, when targeting iOS 11, building for the generic iOS device produces an overflow error. This is strange because iOS 11 only supports 64-bit devices; I think the standard architecture still includes 32-bit, though I cannot figure out how/why.

Changing this hash function and StableId to use an explicit UInt64 avoids the error, and it should not use any more memory, because UInt will always be 64-bit for 64-bit devices anyway.

[codecov[bot]]

Codecov Report

Merging #9 into master will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master       #9   +/-   ##
=======================================
  Coverage   89.55%   89.55%           
=======================================
  Files          22       22           
  Lines         699      699           
=======================================
  Hits          626      626           
  Misses         73       73           

[ctreffs]

Hi @liamdon, thank you so much for using my ECS and actually liking it :-)

Your fix is very appreciated!

I would like to add a step or action to the CI to prevent such problems in the future. Do you have a good idea how we might get that done?

I'm thinking about a 32-bit os as one of the CI targets or is there another way to force 32-bit validation?

[liamdon]

@ctreffs I had a look around, and the easiest approach (if it works) looks like this example: https://github.com/google/benchmark/pull/669/files

i.e:

      env:
        - COMPILER=clang++ BUILD_TYPE=Release BUILD_32_BITS=ON

(^^ btwbenchmark is a really cool project for performance benchmarks, if you haven't tried it already).

If that doesn't work, we could also try using a 32-bit build machine like this: https://stackoverflow.com/a/58409646/5984782

But that's complicated and I don't think it's what we really want - we just want to build for a 32-bit architecture, so hopefully the first example works.

[ctreffs]

Thanks a lot for your input!

Sadly the environment extension is hard to apply to the current setup, since this seems to rely on an objective-c based build. This project currently only uses SPM for all builds and is "pure" Swift. 🤷

Nevertheless I will try to add it to a future CI update. Will migrate to GitHub actions anyways soonish :-)

Thanks for the benchmark tip :-) Will try that!

Regarding the issue: I decided to follow your example and convert all string hashing functions to use UInt64 entirely. The reasoning behind that is, that the string based hashes are used as "stable" hashes and therefore should produce the same results for every platform. They will be the basis for serialization, which is currently in the works. They are not as performance critical als the runtime hash functions, so this assumption should hold.

This should fix all (known) hashing overflows 🤞

Additionally I've ensured that all platforms archive as expected, and using my Xcode it seems so.

Please let me know if this is fixed for you now.

I will close this PR and follow up with all additions in https://github.com/fireblade-engine/ecs/pull/12

Thanks again for your input and help :-)