Skip to content
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

🔡 Unsigned integer 4,8,12,16,20,24 to hexadecimal #2153

Merged
merged 9 commits into from
Oct 12, 2021
Merged

Conversation

CalebSerafin
Copy link
Member

@CalebSerafin CalebSerafin commented Sep 29, 2021

What type of PR is this.

  • Bug
  • Change
  1. Enhancement

What have you changed and why?

Formats a scalar as the specified length hexadecimal string.
These lengths are divided into multiple functions for 4, 8, 12, 16, 20, 24 bits.
This is needed for UUID generation and output. May help other areas.

Please verify the following and ensure all checks are completed.

  1. Have you loaded the mission in LAN host?
  2. Have you loaded the mission on a dedicated server?

Is further testing or are further changes required?

  1. No
  • Yes (Please provide further detail below.)

How can the changes be tested?

[
    A3A_base16LookupTable   # 0x0        isEqualTo "0",
    A3A_base16e2LookupTable # 0xb4       isEqualTo "b4",
    0xaaf       call A3A_fnc_uint12ToHex isEqualTo "aaf",
    0xdead      call A3A_fnc_uint16ToHex isEqualTo "dead",
    0xca1eb     call A3A_fnc_uint20ToHex isEqualTo "ca1eb",
    0xffffff    call A3A_fnc_uint24ToHex isEqualTo "ffffff"
] isEqualTo [true,true,true,true,true,true];

@jaj22
Copy link

jaj22 commented Sep 30, 2021

What part of UUID generation would this be necessary for? I can only see the utility for log output, where it's maybe a bit much given that decimal output would mostly work. I also struggle to see a case where you'd use most of the functions, so one 256-entry lookup table and 16+24 bit output functions seems like plenty of functionality. I don't think this is worth a first-level folder on its own either (think of the poor newbies trying to navigate the codebase), maybe a subfolder under utility using the file = CfgFunctions option.

According to https://community.bistudio.com/wiki/Initialization_Order, preinit is unscheduled anyway so there's no point in the isNil wrapper here.

@CalebSerafin
Copy link
Member Author

For UUIDs longer than the 24bits or made from multiple pieces of information, working with them as strings makes it far easier to stick pieces together. Yeah, I could cut the 4096 entry lookup table. However, the 16 entry one will still be needed to avoid needing to trim zero's afterwards. These are good points.

@CalebSerafin
Copy link
Member Author

Okay, the changes have been made but I'm unable to test as Arma dropped a big S.O.G. update and I don't have the internet for it.

@CalebSerafin CalebSerafin added the Needs testing Needs testing label Sep 30, 2021
@jaj22
Copy link

jaj22 commented Sep 30, 2021

For UUIDs longer than the 24bits or made from multiple pieces of information, working with them as strings makes it far easier to stick pieces together. Yeah, I could cut the 4096 entry lookup table. However, the 16 entry one will still be needed to avoid needing to trim zero's afterwards. These are good points.

Ah, so you're intending to generate with random (maybe in multiple parts) but then convert immediately to string for transfer & storage? That makes sense with SQF.

Are you sure you need all the output functions though? Struggling to see what you'd ever need the short ones for.

@CalebSerafin
Copy link
Member Author

They might be handy. I think that's a fair justification for 3-4 lines per function.

@jaj22
Copy link

jaj22 commented Sep 30, 2021

The mini-functions don't actually benefit from the #include, do they? Seems that they're redundant with the preinit.

@CalebSerafin
Copy link
Member Author

CalebSerafin commented Sep 30, 2021

It's in case it gets called by another pre-init function.
Hmm, okay, I'll move pre-init to the antistasi init and remove the #include.
Edit: Ah no, it should initialise before JIP, just in case it might be used.

@CalebSerafin
Copy link
Member Author

CalebSerafin commented Sep 30, 2021

Edit: Ah no, it should initialise before JIP, just in case it might be used.
Maybe if we add a centralised preinit function (pretty much for preJIP) to Antistasi, the order can be enforced there. What do you think @jaj22?

@jaj22
Copy link

jaj22 commented Sep 30, 2021

That might be necessary in the future, but I think I'm ok with unordered pre-init at the moment, and then create a central function if we need it later.

@CalebSerafin
Copy link
Member Author

@jaj22 pull request approved? Required for merge.

@jaj22
Copy link

jaj22 commented Oct 1, 2021

Weren't you going to remove the #includes? If you're giving me a hard choice between the includes and a centralized ordered preinit then I'll go for the centralized pre-init. For now though I feel like it's fine to have a condition that pre-init doesn't have dependencies on other pre-init.

@CalebSerafin
Copy link
Member Author

@jaj22 Are the new files to your liking. The removal of #include has allowed me to remove functions for uint4 and 8, as they are single lookups.

Environment: Unscheduled
Public: Yes
*/
#include "Includes\common.inc"
Copy link

Choose a reason for hiding this comment

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

Doesn't this one have to be relative in case someone is running the mission unzipped?

Copy link

Choose a reason for hiding this comment

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

Oh, probably needs ..\..\ regardless. initServer.sqf and init.sqf don't need all the double-dot stuff because they're actually in the root.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah copy-paste caught.

@CalebSerafin CalebSerafin removed the request for review from HakonRydland October 1, 2021 16:16
@CalebSerafin CalebSerafin removed the Needs testing Needs testing label Oct 1, 2021
@Bob-Murphy Bob-Murphy added this to the 2.5.X milestone Oct 4, 2021
@Bob-Murphy Bob-Murphy merged commit 0c68128 into unstable Oct 12, 2021
@CalebSerafin CalebSerafin deleted the CS-uintToHex branch October 17, 2021 13:01
@Bob-Murphy Bob-Murphy added the Added to changelog Added to changelog label Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added to changelog Added to changelog Enhancement New feature or request Ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants