Skip to content

Fix deferred lighting Option #6346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading