SubnauticaModding / Nautilus-Proto

Archived prototype for Nautilus, the Subnautica Modding API.
https://subnauticamodding.github.io/Nautilus/
GNU General Public License v3.0
6 stars 3 forks source link

LanguagePatcher Dictionary Error. #16

Closed SamuramongeDev closed 1 year ago

SamuramongeDev commented 1 year ago

Changes made in this pull request

JKohlman commented 1 year ago

I'm a little confused as to your goal here for two reasons:

(1) The dictionary can't be null unless something went terrible wrong as it's a static readonly member

private static readonly Dictionary<string, Dictionary<string, string>> _customLines = new();

(2) How does checking if the dictionary has any arbitrary item in it prevent errors?

If your goal is to just ensure that lines 48 & 49 don't throw an error I'd suggest instead checking to see if the specific objects needed for the function exist in the dictionary (.has(...)) or use TryGetValue (see line 34) to return a boolean and also get the value in the same line.

SamuramongeDev commented 1 year ago

I'm a little confused as to your goal here for two reasons:

(1) The dictionary can't be null unless something went terrible wrong as it's a static readonly member

private static readonly Dictionary<string, Dictionary<string, string>> _customLines = new();

(2) How does checking if the dictionary has any arbitrary item in it prevent errors?

If your goal is to just ensure that lines 48 & 49 don't throw an error I'd suggest instead checking to see if the specific objects needed for the function exist in the dictionary (.has(...)) or use TryGetValue (see line 34) to return a boolean and also get the value in the same line.

When i first launched the game, i saw an error caused by LanguagePatcher.cs trying to acces an item on that dictionary. So, by common sense, i added a check there, and it worked! Currently checking the code.

JKohlman commented 1 year ago

When i first launched the game, i saw an error caused by LanguagePatcher.cs trying to acces an item on that dictionary. So, by common sense, i added a check there, and it worked! Currently checking the code.

In that case I'd suggest using one of the two methods I described instead as they should work and would be a little more accurate to the code.

But also you may need to do a review or get someone more knowledgeable than me to ensure that returning early from that function doesn't break other code

LeeTwentyThree commented 1 year ago

Exiting early without any sort of message/error is never a preferred solution. However I do see potential for error here: https://github.com/SubnauticaModding/Nautilus/blob/aa6a3dffba70159b805d73e222679fc28ad5133a/Nautilus/Patchers/LanguagePatcher.cs#L52-L53

SamuramongeDev commented 1 year ago

Exiting early without any sort of message/error is never a preferred solution. However I do see potential for error here:

https://github.com/SubnauticaModding/Nautilus/blob/aa6a3dffba70159b805d73e222679fc28ad5133a/Nautilus/Patchers/LanguagePatcher.cs#L52-L53

Yeah, so, what we can do in that case? i tested and those cause errors.

LeeTwentyThree commented 1 year ago

The issue is likely rooted elsewhere, and returning early is only a superficial fix.

JKohlman commented 1 year ago

Exiting early without any sort of message/error is never a preferred solution. However I do see potential for error here: https://github.com/SubnauticaModding/Nautilus/blob/aa6a3dffba70159b805d73e222679fc28ad5133a/Nautilus/Patchers/LanguagePatcher.cs#L52-L53

Yeah, so, what we can do in that case? i tested and those cause errors.

Check out how it's done elsewhere in the file, particularly either lines 34 & 35 for the TryGetValue method or lines 94-98 for the Contains method

LeeTwentyThree commented 1 year ago

I can't even figure out what's going on tbh, comments would be much appreciated. @Metious wrote this I believe, so we can wait on him if needed.