Files
Bubberstation/code/modules/unit_tests/objectives.dm
SkyratBot bc2b49f0a5 [MIRROR] Refactors abstract traitor objectives to be more abstract and enforces this by using unit tests. Rebalances some traitor objectives [MDB IGNORE] (#19745)
* Refactors abstract traitor objectives to be more abstract and enforces this by using unit tests. Rebalances some traitor objectives (#73777)

## About The Pull Request
In this PR, some of the traitor objectives are rebalanced to be more
consistent with scaling risk.
Abstract traitor objectives have had their telecrystal reward and
progression rewards moved to their non-abstract types and it's now
enforced that abstract objectives should not have these values set.
Additionally, it's encouraged that people don't set progression_minimum
on abstract types either, but I can see the usecase in doing so with
final objectives and assassinate objectives. This is why it's fine to
set progression_minimum on an abstract type as long as any of the
derivatives of that abstract type do not redefine the progression
minimum to avoid consistency errors when tweaking progression minimum
values. Setting the progression minimum on an abstract type means that
all derivatives of that abstract type should be unlocked at roughly the
same time.

## Why It's Good For The Game
The rebalances are so that same risk objectives of different types are
worth around the same amount. Repeatables should roughly award the same
amount of TC when it comes to comparing the risk, but the progression
rewards can vary.
The new standard enforcement on abstract traitor objectives is more so
for robustness and ease of balance, as it's easier to lose consistency
when rebalancing values between two objectives, because one of the
objectives derive their rewards from an abstract type. Generally
speaking, rewards from objectives of different risk level should not be
the same and it's easier to enforce this if developers have to
explicitly declare the rewards of the objectives they add.
This doesn't mean each objective has to declare explicitly what their
reward is. Derivatives that subtype off of non-abstract types can still
copy the rewards from their parent.

The progression minimum is fine to be set on abstract objectives as long
as derivatives don't change the progression minimum. If they do, then
it's better for consistency to declare the progression minimum on each
type rather than the abstract type so that higher-tier objectives don't
accidentally end up with a lower progression minimum when it comes to
rebalancing. Of course, this isn't a set rule on, but it's something I'm
going to try and enforce, when it makes sense, going forward, even if it
may increase the number of lines of code each traitor objective file may
have. Maintainability and robustness beat optimization.

## Changelog
🆑
code: Abstract types don't hold telecrystal rewards or progression
rewards anymore, this has been moved to the non-abstract types.
balance: Rebalances rewards from repeatable traitor objectives to be
more consistent with each other.
/🆑

---------

Co-authored-by: Watermelon914 <3052169-Watermelon914@ users.noreply.gitlab.com>

* Refactors abstract traitor objectives to be more abstract and enforces this by using unit tests. Rebalances some traitor objectives

---------

Co-authored-by: Watermelon914 <37270891+Watermelon914@users.noreply.github.com>
Co-authored-by: Watermelon914 <3052169-Watermelon914@ users.noreply.gitlab.com>
2023-03-09 20:08:27 +00:00

50 lines
3.3 KiB
Plaintext

/datum/unit_test/objectives_category/Run()
var/datum/traitor_category_handler/category_handler = allocate(/datum/traitor_category_handler)
var/list/objectives_that_exist = list()
for(var/datum/traitor_objective_category/category as anything in category_handler.all_categories)
for(var/value in category.objectives)
TEST_ASSERT(isnum(category.objectives[value]), "[category.type] does not have a valid format for its objectives as an objective category! ([value] requires a weight to be assigned to it)")
if(islist(value))
recursive_check_list(category.type, value, objectives_that_exist)
else
objectives_that_exist += value
SStraitor.generate_objectives = FALSE
for(var/datum/traitor_objective/objective_typepath as anything in subtypesof(/datum/traitor_objective))
var/datum/traitor_objective/objective = allocate(objective_typepath)
if(objective.abstract_type == objective_typepath)
// In this case, we don't want abstract types to define values that should be defined on non-abstract types
// Nor do we want abstract types to appear in the pool of traitor objectives.
if(objective_typepath in objectives_that_exist)
TEST_FAIL("[objective_typepath] is in a traitor category and is an abstract type! Please remove it from the [/datum/traitor_objective_category].")
// Since we didn't generate the objective, the rewards are going to be in list form: (min, max)
if(!reward_is_zero(objective.progression_reward))
TEST_FAIL("[objective_typepath] has set a progression reward as an abstract type! Please define progression rewards on non-abstract types rather than abstract types.")
// Since we didn't generate the objective, the rewards are going to be in list form: (min, max)
if(!reward_is_zero(objective.telecrystal_reward))
TEST_FAIL("[objective_typepath] has set a telecrystal reward as an abstract type! Please define telecrystal rewards on non-abstract types rather than abstract types.")
continue
if(!(objective_typepath in objectives_that_exist))
TEST_FAIL("[objective_typepath] is not in a traitor category and isn't an abstract type! Place it into a [/datum/traitor_objective_category] or remove it from code.")
if(objective.progression_minimum == null)
TEST_FAIL("[objective_typepath] has not defined a minimum progression level and isn't an abstract type! Please define the progression minimum variable on the datum")
if(objective.needs_reward && reward_is_zero(objective.progression_reward) && reward_is_zero(objective.telecrystal_reward))
TEST_FAIL("[objective_typepath] has not set either a progression reward or a telecrystal reward! Please set either a telecrystal or progression reward for this objective.")
/// Returns whether the reward specified (in format (min, max)) is zero or not.
/datum/unit_test/objectives_category/proc/reward_is_zero(list/reward)
return (reward[1] == 0 && reward[2] == 0)
/datum/unit_test/objectives_category/Destroy()
SStraitor.generate_objectives = TRUE
return ..()
/datum/unit_test/objectives_category/proc/recursive_check_list(base_type, list/to_check, list/to_add_to)
for(var/value in to_check)
TEST_ASSERT(isnum(to_check[value]), "[base_type] does not have a valid format for its objectives as an objective category! ([value] requires a weight to be assigned to it)")
if(islist(value))
recursive_check_list(base_type, value, to_add_to)
else
to_add_to += value