* resets state updater to apply light settings (#3141)
resets state updater to apply light settings
With this PR we achieve the same effect with fewer lines of code.
* fixes LightSource logic errors
We currently update `LightSource::setActorFade` in `TransparencyUpdater`. There are several logic errors inherent in this approach:
1. We fail to update `LightSource::setActorFade` for off screen actors because their `TransparencyUpdater` cull callback is not invoked.
2. We fail to update `LightSource::setActorFade` in the instant that a `TransparencyUpdater` is removed.
3. We fail to update `setActorFade` when an `mExtraLightSource` is created after calling `Animation::setAlpha`.
With this PR we avoid such issues by updating `LightSource::setActorFade` in `Animation::setAlpha` and `Animation::addExtraLightSource` instead.
With this PR we refactor `SceneUtil::KeyframeController` not to require `virtual osg::Callback` inheritance. I suppose such `virtual` overhead is not justified here because it negatively impacts many other classes we derive from `osg::Callback`.
This PR aims to spark the retirement of a questionable pattern I have found all over our code base. I will illustrate how this pattern encourages code duplication, lacks type safety, requires documentation and can be prone to bugs.
```
std::map<std::string, Object> mMap; // Stored in all lowercase for a case-insensitive lookup
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.emplace(lowerKey, object);
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.find(lowerKey);
mMap.find(key); // Not found. Oops!
```
An alternative approach produces no such issues.
```
std::unordered_map<std::string, Object, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual> mMap;
mMap.emplace(key, object);
mMap.find(key);
```
Of course, such an alternative will work for a `map` as well, but an `unordered_map` is generally preferable over a `map` with these changes because we have moved `lowerCase` into the comparison operator.
In this PR I have refactored `Animation::mNodeMap` accordingly. I have reviewed and adapted all direct and indirect usage of `Animation::mNodeMap` to ensure we do not change behaviour with this PR.
This PR fixes a crash caused by the improperly ensured lifetime of RigGeometry::mSourceGeometry. mSourceGeometry was not adequate to ensure mSourceGeometry would outlive mGeometry because we extend mGeometrys lifetime beyond this lifetime by passing mGeometry to the draw traversal instead of this.
In addition,
We add important comments.
We detect and prevent generally unsafe operations in high level code.
We add a sprinkling of const to help clarify intentions.
Currently, we use a peculiar mapping of ESM classes by their std::type_info::name. This mapping is an undefined behaviour because std::type_info::name is strictly implementation defined. It could return a non-unique value on some platforms. With this PR we use the unsigned int sRecordId of the ESM class as a more efficient lookup type that does not build on undefined behaviour. We can expect marginally faster save-game loading with these changes as well.
