Fix an issue with sandworms being always instantiated to HOUSE_HARKONNEN.

[juj]

Fixes #29.

Before this change, Sand worms would be created to HOUSE_HARKONNEN.

After this change, Sand worms are created to HOUSE_FREMEN like the .ini assets suggest, e.g.

ID016=Fremen,Sandworm,256,2641,64,Ambush
ID017=Fremen,Sandworm,256,2210,64,Ambush

Before Fremen would be associated with HOUSE_HARKONNEN:

Afterwards, Sand worms are instantiated to HOUSE_FREMEN:

With some luck, this might also fix issue #15.

It might also fix the issue that on my playthroughs of each of Harkonnen, Ordos and Atreides, I never saw any Sardaukar dropship troops during the game. I recall that during the campaign in later levels, the Emperor should be supplying the enemy with some Sardaukar troops, but that never happened.

[henricj]

The oldest version of that function in the repo is this, so the behavior has been the same for a long time:

HOUSETYPE INIMapLoader::getHouseID(std::string name) {
    convertToLower(name);

    if(housename2house.count(name) > 0) {
        return housename2house[name];
    } else {
        return getHouseByName(name);
    }
}

Is creating a new entry in housename2house with name name and value HOUSE_HARKONNEN (0) deliberate? If not, the result of the find() in the PR might as well be used instead of doing the lookup again:

HOUSETYPE INIMapLoader::getHouseID(std::string_view name) {
    const auto lowerName = strToLower(name);

    const auto it = housename2house.find(lowerName);
    if (it != housename2house.end())
        return it->second;

    return getHouseByName(lowerName);
}

If it should be updated, then presumably the value returned from getHouesByName() should be used, not HOUSETYPE{}.

[juj]

Is creating a new entry in housename2house with name name and value HOUSE_HARKONNEN (0) deliberate?

Having "fremen" there mapping to HOUSE_HARKONNEN via an implicit default creation of a map element on [] read access does clearly read like a bug. The intent of that housename2house mapping seems to map string to their house names, not "map to house name, or default-to-harkonnen".

Without this change, if one is playing Harkonnen (and not Ordos or Atreides):

• all sand worms are immediately revealed on the map at start of level,
• one can double click on a sand worm to multiselect all other sand worms in the level,
• if an enemy attacks a sand worm, the attacker is revealed on the map

These quirks went away after this PR change.

the result of the find() in the PR might as well be used instead of doing the lookup again:

That makes sense, updated to that form.

At first I thought that the role of housename2house was a simple micro-optimization to avoid doing string comparisons again and again when searching for the mapping to HOUSETYPE IDs. Although then I found this logic:

https://github.com/henricj/dunelegacy/blob/b66573fe2ebfc63ce6a52421bda9f4c6f8b6f3a5/src/INIMap/INIMapLoader.cpp#L392-L396

which does initialize mappings "ordos" -> HOUSE_UNUSED and "harkonnen" -> HOUSE_UNUSED at first, and then they slightly later get updated to actual HOUSETYPE mappings:

https://github.com/henricj/dunelegacy/blob/b66573fe2ebfc63ce6a52421bda9f4c6f8b6f3a5/src/INIMap/INIMapLoader.cpp#L433

I don't know why all the hoops for that, maybe it has something to do with the map data format - or maybe really leftover of something else.

One thing that is notable is that logic does not create a mapping "fremen" -> HOUSE_UNUSED in the housename2house e.g. when there are sand worms in the level, so it seems somehow intentional(?) that the housename2house would only contain mappings of the houses that are actual active player/equipped-with-base factions in the level.

(although I can't find if there are any uses of that housename2house map that would be looking for HOUSE_UNUSED vs something else..)

If that was not the case, I would have interpreted the intent of getHouseID to be a kind of a lazy caching mapping optimization, i.e. it would have ideally wanted to be of form

HOUSETYPE INIMapLoader::getHouseID(std::string_view name) {
    const auto it = housename2house.find(name);
    if (it != housename2house.end())
        return it->second;
    auto house = getHouseByName(strToLower(name));
    housename2house[name] = house;
    return house;
}

but in the current form, the logic is something a bit different since Fremen (and one-off Sardaukar?) units are not placed there.