Harmony.CreateAndPatchAll - ensure no duplicate guids

[ManlyMarco]

https://github.com/BepInEx/HarmonyX/blob/master/Harmony/Public/Harmony.cs#L367

There exists a world where Guid.NewGuid() returns the same thing twice for different instances, and it creates a weird bug someone wastes time chasing.

How about adding a check for existing instances and generate a new guid if it already exists?

Alternatively it could use assembly-qualified type name instead. As a bonus it would be much more descriptive when debugging (it would need a dupe check as well).

[ghorsington]

Have you managed to replicate this issue specifically? According to MSDN on Guid.NewGuid():

On Windows, this function wraps a call to the CoCreateGuid function. The generated GUID contains 122 bits of strong entropy.

Moreover, on Unity mono (anything prior to 2020) NewGuid actually uses RandomNumberGenerator to create the GUID (source) which actually should produce much better randomness.

If I'd fix anything, I'd probably replace the whole GUID with a direct call to RandomNumberGenerator to generate a random string.

[ManlyMarco]

It's just theoretical. Using RandomNumberGenerator can still result in getting two of the same value. The odds are astronomical but I'd feel a lot better using this feature if there was no chance of this happening at all.

[KryziK]

While technically possible, it's called a GUID (globally unique identifier) for a reason. GUIDs are used so that you DON'T have to check them.

So many systems and companies rely on a GUID being unique that we'd have bigger problems if it ever were not.

There is basically no need to check for a duplicate GUID in a worldwide database, let alone a plugin that is likely to be among less than 100 other plugins running in the same process.

This is a convenient static method that you can call to get a new Guid. The method creates a Version 4 Universally Unique Identifier (UUID) as described in RFC 4122, Sec. 4.4. The returned Guid is guaranteed to not equal Guid.Empty. On Windows, this function wraps a call to the CoCreateGuid function. The generated GUID contains 122 bits of strong entropy.

The CoCreateGuid function calls the RPC function UuidCreate, which creates a GUID, a globally unique 128-bit integer. Use CoCreateGuid when you need an absolutely unique number that you will use as a persistent identifier in a distributed environment. To a very high degree of certainty, this function returns a unique value – no other invocation, on the same or any other system (networked or not), should return the same value.

While each generated GUID is not guaranteed to be unique, the total number of unique keys (2^128 or 3.4×10^38) is so large that the probability of the same number being generated twice is very small. For example, consider the observable universe, which contains about 5×10^22 stars; every star could then have 6.8×10^15 universally unique GUIDs.

---

There exists a world where Guid.NewGuid() returns the same thing twice for different instances, and it creates a weird bug someone wastes time chasing.

As for this, perhaps you mean universe ;). And if someone were to run into that "bug", that would be beyond wild to say the least. The odds of winning the lottery are less than 3×10^8. You'd be more likely to win the lottery 1×10^30 times than get 2 GUIDs that were the same (is that how exponents work? idk).

Regardless, we could chase down theoretical things that will never happen forever, but what is the point?

[ManlyMarco]

Well yes, in practice this is not going to happen, no one's disputing that. I still think it would be worthwhile to at least add the calling assembly name to the generated guid for easier debugging.

[KryziK]

For what it's worth, you could pass your own name/GUID to both of these for easier debugging:

Harmony.CreateAndPatchAll(..., nameHere)
new Harmony.Harmony(nameHere)