fireblade-engine / ecs

A dependency free, lightweight, fast Entity-Component System (ECS) implementation in Swift
MIT License
110 stars 11 forks source link

Fix hashing overflow - follow up #12

Closed ctreffs closed 4 years ago

ctreffs commented 4 years ago

This PR is a follow up to https://github.com/fireblade-engine/ecs/pull/9.

Fixes:

codecov[bot] commented 4 years ago

Codecov Report

Merging #12 into master will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #12      +/-   ##
==========================================
- Coverage   89.55%   89.54%   -0.02%     
==========================================
  Files          22       22              
  Lines         699      698       -1     
==========================================
- Hits          626      625       -1     
  Misses         73       73              
ctreffs commented 4 years ago

@liamdon would be great if you could verify that this PR fixes your issue 🙏 thanks

liamdon commented 4 years ago

Yes, fixed on our project!

For additional confirmation, I tried building on a Raspberry Pi docker image (which is 32-bit ARM, similar to old iPhones), comparing master and this branch.

Here is the result on master:

root@5944a76b1d08:/ecs# git branch
* master
root@5944a76b1d08:/ecs# swift build
/ecs/Sources/FirebladeECS/Hashing.swift:97:34: error: integer literal '72057594037927935' overflows when stored into 'UInt'
            hash = 127 * (hash & 0x00ffffffffffffff) + UInt(char.value)
                                 ^
/ecs/Sources/FirebladeECS/Hashing.swift:97:34: error: integer literal '72057594037927935' overflows when stored into 'UInt'
            hash = 127 * (hash & 0x00ffffffffffffff) + UInt(char.value)
                                 ^
/ecs/Sources/FirebladeECS/Hashing.swift:97:34: error: integer literal '72057594037927935' overflows when stored into 'UInt'
            hash = 127 * (hash & 0x00ffffffffffffff) + UInt(char.value)
                                 ^
/ecs/Sources/FirebladeECS/Hashing.swift:97:34: error: integer literal '72057594037927935' overflows when stored into 'UInt'
            hash = 127 * (hash & 0x00ffffffffffffff) + UInt(char.value)

And here with this branch:

root@5944a76b1d08:/ecs# git branch
* feature/classdojo_fixHashingOverflow
  master
root@5944a76b1d08:/ecs# swift build
[26/26] Merging module FirebladeECS
root@5944a76b1d08:/ecs#

So, I think you can consider it fixed 👍

ctreffs commented 4 years ago

Thank you so much! That is a relief! ✅

As a side node I would be very interested in your Raspberry Pi project setup :-) If you're interested in talking more about that you let's talk on Twitter (https://twitter.com/ChrisDailyGrind) or by some other means than pull requests 😄

Thanks again for your contribution - anytime again if you like 👍