zolantris / ValheimMods

Collection of ValheimMods, ValheimRaft (originally created by Sarcen) and a few other upcoming mods including a vehicles mod.
GNU General Public License v3.0
11 stars 5 forks source link

A small error in the code, in the localization key. #126

Closed lijspoop closed 2 weeks ago

lijspoop commented 1 month ago

https://github.com/zolantris/ValheimMods/blob/6ea46882655e5264d8e48eed2c23ec0676bee702/src/ValheimVehicles/ValheimVehicles.Prefabs/PrefabRegistryHelpers.cs#L309

Missing $ at the beginning of the string

$"$valheim_vehicles_hull_center_desc {ironMatDesc}"

zolantris commented 1 month ago

Thanks, I believe it's fixed in a unstable branch. But at least we can track it here now. Good catch!

I'll try to get this fixed in next update.

Long term hoping to add translation injection to parse the json file to see if a dollar sign is missing for something that looks like a key.

Would be a "build" test.

lijspoop commented 1 month ago

The same mistake image

https://github.com/zolantris/ValheimMods/blob/6ea46882655e5264d8e48eed2c23ec0676bee702/src/ValheimVehicles/ValheimVehicles.Prefabs/PrefabRegistryHelpers.cs#L83-L85


And while reviewing the code, I had two questions:

  1. Why use a ternary operator in line 215, since there is no such check in line 217? https://github.com/zolantris/ValheimMods/blob/6ea46882655e5264d8e48eed2c23ec0676bee702/src/ValheimVehicles/ValheimVehicles.Prefabs/Registry/SailPrefabs.cs#L213-L219

I think that line 215 can be deleted (by removing the unnecessary creation of a variable), and it should turn out something like this:

 var piece = prefab.AddComponent<Piece>(); 
 piece.m_name = $"$mb_sail_{sailCount}"; 
 piece.m_description = $"$mb_sail_{sailCount}_desc"; 
 piece.m_placeEffect = LoadValheimAssets.woodFloorPiece.m_placeEffect; 
  1. Also, why use "mb_sail" if it is already used by another method (RegisterCustomSail) of creating a sail, moreover, it is not found anywhere in the release? https://github.com/zolantris/ValheimMods/blob/6ea46882655e5264d8e48eed2c23ec0676bee702/src/ValheimVehicles/ValheimVehicles.Prefabs/Registry/SailPrefabs.cs#L174-L175

Most likely, this can be seen somewhere else in the code, but it doesn't hurt to ask at least about it.

lijspoop commented 1 month ago

Long term hoping to add translation injection to parse the json file to see if a dollar sign is missing for something that looks like a key.

Would be a "build" test.

Is it possible to do it differently? For example, generate a translation file when "build".

lijspoop commented 3 weeks ago

try using this method to avoid something like this; it adds a $ symbol to the beginning of a translation key if it isn't already there. For example, the string "valheim_vehicles_ram_stake valheim_vehicles_material_tier3" will be transformed into "$valheim_vehicles_ram_stake $valheim_vehicles_material_tier3".

can be checked here

    public static string NormalizeTranslationKeys(string localizableString)
    {
      return new Regex(@"((?<!\$)\b\w+_\w+\b)").Replace(localizableString, match => "$" + match.Value);
    }

for example, you can use it here: https://github.com/zolantris/ValheimMods/blob/ed67ed1f3d93c70bafdd4e0f55f16d34011fa44b/src/ValheimVehicles/ValheimVehicles.Prefabs/PrefabRegistryHelpers.cs#L20-L25

by doing this:

public struct PieceData
{
    private string _name;
    private string _description;

    public string Name
    {
        get => _name;
        set => _name = NormalizeTranslationKeys(value);
    }

    public string Description
    {
        get => _description;
        set => _description = NormalizeTranslationKeys(value);
    }

    public Sprite Icon;
}