Paradise icon indicating copy to clipboard operation
Paradise copied to clipboard

Refactors objective code to account for there being multiple owners (team objectives)

Open SteelSlayer opened this issue 2 years ago • 1 comments

I highly recommend that this PR be test merged for a while.

What Does This PR Do

Refactors objective code to account for there being multiple owners (team objectives). This needs to happen before I can make my antag team framework PR.

Currently, a check_completion() proc for an objective might look something like this:

/datum/objective/my_obj/check_completion()
	if(owner.current.stat == DEAD)
		return FALSE
	return TRUE

Under the new system, we need to account for an objective having multiple owners, so we need to use the get_owners() proc to grab ALL owners, including the ones from a team.

/datum/objective/my_obj/check_completion()
	for(var/datum/mind/M in get_owners())
		if(M.current.stat == DEAD)
			return FALSE
	return TRUE

This doesn't apply to just check_completion. This applies to anywhere where you're checking some condition on the objective owner(s). Also note that it's fine to use owner over get_owners() if we're certain that objective will never belong to a team.

Additionally cleans up some things:

  • Removes /datum/objective/download. This is barebones and doesn't even have code that allows the owner to succeed.
  • Removes /datum/objective/capture, for the same reason.
  • Removes /datum/objective/steal/exchange. These objectives haven't been used for years.

Testing

Loaded up a test server and ran check_completion() on all objectives that I could. Still, testing each objective completion in depth is annoying and time consuming, so I still want a TM for this PR.

Changelog

Should have no player-facing changes.

SteelSlayer avatar Oct 05 '22 01:10 SteelSlayer

this makes me very happy :>

GDNgit avatar Oct 05 '22 03:10 GDNgit

Any runtimes or issues relating to this? I note there has been an uptick in free objectives

S34NW avatar Nov 14 '22 13:11 S34NW

@S34NW I observed a couple rounds and didn't see any issues, but it needs TMing for an extended amount of time to get more data. Also I'm aware of the Free objective thing, and I need to fix that, but it's unrelated to this PR. This PR only adds support for multiple owners to objectives.

SteelSlayer avatar Nov 14 '22 15:11 SteelSlayer

The free objective thing was caused by this PR. Fixed in the latest commit by removing a single !.

SteelSlayer avatar Nov 16 '22 00:11 SteelSlayer

Fixed a bug where if you have the escape objective, you're off station, and the AI doomsdays/station is nuked, the objective would fail.

SteelSlayer avatar Nov 18 '22 00:11 SteelSlayer