Skip to content

Commit

Permalink
Fix deferred lighting Option (#6346)
Browse files Browse the repository at this point in the history
* Fix deferred lighting option

* Fix issue

* Fix premature cache

* Appease Clang

* cache specific option not options in general
  • Loading branch information
BMagnu authored Sep 9, 2024
1 parent 45d9926 commit b6879da
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 15 deletions.
3 changes: 2 additions & 1 deletion code/lighting/lighting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,13 @@ static auto DeferredLightingOption = options::OptionBuilder<bool>("Graphics.Defe
bool light_deferred_enabled()
{
if (Using_in_game_options) {
static bool isToggledOn = DeferredLightingOption->getValue();
// This used to be getting the value of the option object itself,
// however that is not a free operation and changing it requires a restart anyway
// if the restart requirement is lifted care should be taken to cache this value
// and never look it up more than once a frame
// otherwise the performance footprint is measurable enough to worry about.
return DeferredLightingEnabled;
return isToggledOn;
} else {
return !Cmdline_no_deferred_lighting;
}
Expand Down
8 changes: 8 additions & 0 deletions code/options/Option.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,14 @@ void OptionBase::setFlags(const flagset<OptionFlags>& flags) {
_flags = flags;
}

bool OptionBase::getIsOnce() const {
return _is_once;
}

void OptionBase::setIsOnce(bool is_once) {
_is_once = is_once;
}

//persists any changes made to this specific option and returns whether or not it was successful
bool OptionBase::persistChanges() const { return _parent->persistOptionChanges(this); }

Expand Down
15 changes: 10 additions & 5 deletions code/options/Option.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class OptionBase {
float _min = 0;
float _max = 1;

bool _is_once = false;

SCP_unordered_map<PresetKind, SCP_string> _preset_values;

flagset<OptionFlags> _flags;
Expand All @@ -100,6 +102,9 @@ class OptionBase {
const flagset<OptionFlags>& getFlags() const;
void setFlags(const flagset<OptionFlags>& flags);

bool getIsOnce() const;
void setIsOnce(bool is_once);

const SCP_string& getConfigKey() const;
const SCP_string& getTitle() const;
const SCP_string& getDescription() const;
Expand Down Expand Up @@ -519,6 +524,7 @@ class OptionBuilder {
//The global variable to bind the option to once. Will require game restart to persist changes.
OptionBuilder& bind_to_once(T* dest)
{
_instance.setIsOnce(true);
return change_listener([dest](const T& val, bool initial) {
if (initial) {
*dest = val;
Expand Down Expand Up @@ -555,13 +561,13 @@ class OptionBuilder {
return *this;
}
//Finishes building the option and returns a pointer to it
const Option<T>* finish()
std::shared_ptr<const Option<T>> finish()
{
for (auto& val : _preset_values) {
_instance.setPreset(val.first, json_dump_string_new(_instance.getSerializer()(val.second),
JSON_COMPACT | JSON_ENSURE_ASCII | JSON_ENCODE_ANY));
}
std::unique_ptr<Option<T>> opt_ptr(new Option<T>(_instance));
auto opt_ptr = make_shared<Option<T>>(_instance);

if (mpark::holds_alternative<std::pair<const char*, int>>(_title)) {
const auto& xstr_info = mpark::get<std::pair<const char*, int>>(_title);
Expand All @@ -573,9 +579,8 @@ class OptionBuilder {
lcl_delayed_xstr(opt_ptr->_description, xstr_info.first, xstr_info.second);
}

auto ptr = opt_ptr.get(); // We need to get the pointer now since we loose the type information otherwise
OptionsManager::instance()->addOption(std::unique_ptr<OptionBase>(opt_ptr.release()));
return ptr;
OptionsManager::instance()->addOption(opt_ptr);
return opt_ptr;
}
};

Expand Down
10 changes: 5 additions & 5 deletions code/options/OptionsManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void OptionsManager::setOverride(const SCP_string& key, const SCP_string& json)
}

//Adds an option to the options vector
const OptionBase* OptionsManager::addOption(std::unique_ptr<const OptionBase>&& option)
const OptionBase* OptionsManager::addOption(std::shared_ptr<const OptionBase>&& option)
{
_options.emplace_back(std::move(option));
_optionsSorted = false; // Order got invalidated by adding a new option
Expand All @@ -106,12 +106,12 @@ const OptionBase* OptionsManager::addOption(std::unique_ptr<const OptionBase>&&
}

//Removes an option from the options vector
void OptionsManager::removeOption(const OptionBase* option)
void OptionsManager::removeOption(const std::shared_ptr<const OptionBase>& option)
{
_optionsMapping.erase(option->getConfigKey());
_options.erase(
std::remove_if(_options.begin(), _options.end(),
[option](const std::unique_ptr<const OptionBase>& ptr) { return ptr.get() == option; }));
[option](const std::shared_ptr<const OptionBase>& ptr) { return ptr == option; }));
}

// Returns an option with the specified name
Expand All @@ -126,13 +126,13 @@ const OptionBase* OptionsManager::getOptionByKey(SCP_string key)
}

//Returns a table of all built-in options available
const SCP_vector<std::unique_ptr<const options::OptionBase>>& OptionsManager::getOptions()
const SCP_vector<std::shared_ptr<const options::OptionBase>>& OptionsManager::getOptions()
{
if (!_optionsSorted) {
// Keep options sorted by only sorting them when necessary

std::sort(_options.begin(), _options.end(),
[](const std::unique_ptr<const OptionBase>& left, const std::unique_ptr<const OptionBase>& right) {
[](const std::shared_ptr<const OptionBase>& left, const std::shared_ptr<const OptionBase>& right) {
return *left < *right;
});

Expand Down
8 changes: 4 additions & 4 deletions code/options/OptionsManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class OptionsManager {

SCP_unordered_map<SCP_string, const OptionBase*> _optionsMapping;

SCP_vector<std::unique_ptr<const OptionBase>> _options;
SCP_vector<std::shared_ptr<const OptionBase>> _options;
bool _optionsSorted = false;

public:
Expand All @@ -33,13 +33,13 @@ class OptionsManager {

void setOverride(const SCP_string& key, const SCP_string& json);

const OptionBase* addOption(std::unique_ptr<const OptionBase>&& option);
const OptionBase* addOption(std::shared_ptr<const OptionBase>&& option);

void removeOption(const OptionBase* option);
void removeOption(const std::shared_ptr<const OptionBase>& option);

const OptionBase* getOptionByKey(SCP_string name);

const SCP_vector<std::unique_ptr<const options::OptionBase>>& getOptions();
const SCP_vector<std::shared_ptr<const options::OptionBase>>& getOptions();

bool persistOptionChanges(const options::OptionBase* option);

Expand Down

0 comments on commit b6879da

Please sign in to comment.