Skip to content

Main - Replace Fast Hash system with native hashmaps #1320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

JonBons
Copy link
Contributor

@JonBons JonBons commented Apr 1, 2024

When merged this pull request will:

  • Replaces all fast hash based macros with native HashMap methods
  • Removes the fast hash monitor and fast hash garbage collector
  • Removes ACRE_FastHashNamespaceDummy location type as it's unnecessary now

JonBons added 2 commits March 31, 2024 21:09
- removes the garbage collector and hash monitor as these are no longer necessary
- no longer necessary with native hashmaps
@JonBons JonBons changed the title Main - Replaced Fast Hash system with native hashmaps Main - Replace Fast Hash system with native hashmaps Apr 1, 2024
@jonpas jonpas added this to the 2.13.0 milestone Apr 1, 2024
@jonpas jonpas requested a review from PabstMirror April 1, 2024 02:32
@jonpas jonpas merged commit 1385ce4 into IDI-Systems:master Apr 3, 2024
@@ -1,13 +0,0 @@
class CfgLocationTypes {
// For use in Fast Hashes ONLY!
class ACRE_FastHashNamespaceDummy {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to leave this in for one release if it could effect saved games

@PabstMirror
Copy link
Collaborator

need to check capitalization on all hash keys
e.g. "channelMode" vs "channelmode" on 117/152

["ACRE_PRC117F_ID_1"] call acre_sys_prc117f_fnc_getChannelDataInternal
now returns nil

["ACRE_PRC152", "default"] call acre_api_fnc_getPresetData;
returned location now hashmap

["ACRE_PRC152_ID_1"] call acre_api_fnc_getRadioChannel; // item's config case
returns -1
["acre_prc152_id_1"] call acre_api_fnc_getRadioChannel;
returns expected 6

@BrettMayson
Copy link
Contributor

BrettMayson commented Apr 20, 2024

["ACRE_PRC152", "default"] call acre_api_fnc_getPresetData;
returned location now hashmap

This is quite a problem for other mods is it not? There is for example no documented way to get the channel labels except via this function, followed up with getVariable, this is going to break as it now will need hashmap functions

@veteran29
Copy link
Contributor

veteran29 commented Apr 21, 2024

Yep, code like this will break... But it never was a public API so I guess fair enough.

@BrettMayson
Copy link
Contributor

I mean the example I posted is very much public api

@JonBons
Copy link
Contributor Author

JonBons commented Apr 22, 2024

I think there is enough complications from this with mod compat and general API use that it should probably be reverted until a better solution is designed or you're willing to break compat.

I don't use any public mods that interface with ACRE2 so I was unaware people were directly accessing this data and not utilizing some form of API call that could have adjustments made to maintain compat at the tradeoff of slightly worse perf per call (toLowerANSI on keys, etc)

@jonpas
Copy link
Member

jonpas commented Jun 13, 2024

We will revert if necessary (#1322 does not make it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants