search

ThatOpen / engine_web-ifc

Reading and writing IFC files with Javascript, at native speeds.
https://thatopen.github.io/engine_web-ifc/demo
Mozilla Public License 2.0
617 stars 190 forks source link

CreateIfcGuidToExpressIdMapping looks broken #285

Closed pablo-mayrgundter closed 1 year ago

pablo-mayrgundter commented 1 year ago

I asked if anyone was concerned about this on discord.. and actually looks like I may be the only one using it.

https://discord.com/channels/799990228336115742/799990228336115745/1052364020109299793 https://github.com/search?q=CreateIfcGuidToExpressIdMapping&type=code

It's doing a counter from 0 to numEntities, not actually mapping over the Entity map.. I'm not really familiar with TS.. maybe it's a clever way to iterate over object keys? Doesn't seem so.

Either way, seems like the right way to do this is to iterate over the types in the current model, not over all possible types.

I'm working on a local fix

pablo-mayrgundter commented 1 year ago

@beachtom moving convo here

It looks like the current method is using array indexing as a hold-over from the old approach: https://github.com/IFCjs/web-ifc/blame/main/src/web-ifc-api.ts#L539

vs https://github.com/IFCjs/web-ifc/blob/69b14f8c6b27bb49cbcfada0a16e6fc241dd0ab0/src/web-ifc-api.ts#L411

PR sent: https://github.com/IFCjs/web-ifc/pull/286

pablo-mayrgundter commented 1 year ago

::take

pablo-mayrgundter commented 1 year ago

I think this is fixed. PR got merged

On Wed, Dec 14, 2022, 3:17 PM Pablo Mayrgundter @.***> wrote:

::take

— Reply to this email directly, view it on GitHub https://github.com/IFCjs/web-ifc/issues/285#issuecomment-1352213623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS5V34NMQGPZPVNZ3VWFW3WNI2OPANCNFSM6AAAAAAS52SXDI . You are receiving this because you are subscribed to this thread.Message ID: @.***>