From e5f023a3d53ffe309de544adddc40b7dbd7b97d0 Mon Sep 17 00:00:00 2001
From: Bloop <13398309+vinylspiders@users.noreply.github.com>
Date: Fri, 27 Sep 2024 22:19:34 -0400
Subject: [PATCH] Fixes polling causing an unexpected return value when there
 are 0 players picked (#86912)

## About The Pull Request

So this was caused by the change in
https://github.com/tgstation/tgstation/pull/86012 .

They tried to fix a case for polls with `amount_to_pick` set to _greater
than 1_ having null entries in the returned lists, and succeeded at
that.

However, in doing so they messed up the control flow of the proc such
that now it's returning early with an empty `list()` whenever the amount
is _equal to 1_.

The assumption was, when `amount_to_pick` was equal to 1 specifically
then the return value would be either a `null` value or the valid mob.
However, now what was being returned was either a `list()` or the valid
mob because that code was no longer ever being reached.

For example


![image](https://github.com/user-attachments/assets/e49fe031-7120-4a17-8bba-3c5054a90fae)


![image](https://github.com/user-attachments/assets/c7ae27a8-1dce-4fcd-9f19-fcde417f7695)

Fixes https://github.com/NovaSector/NovaSector/issues/4427

Fixes both bugs in a way that should not cause any problems, and adjusts
documentation to make it clearer what the return value of this proc is
expected to be.

## Why It's Good For The Game

Fixes a bug that pretty much effects the majority of ghost polls as most
of them are only asking for 1 player.

## Changelog

:cl:
fix: fixes pulsing tumor failing to spawn the elite if no ghosts respond
to the poll and leaving you in a bugged state, and possibly other
related issues
/:cl:

<!-- Both :cl:'s are required for the changelog to work! You can put
your name to the right of the first :cl: if you want to overwrite your
GitHub username as author ingame. -->
<!-- You can use multiple of the same prefix (they're only used for the
icon ingame) and delete the unneeded ones. Despite some of the tags,
changelogs should generally represent how a player might be affected by
the changes rather than a summary of the PR's contents. -->
---
 code/controllers/subsystem/polling.dm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/code/controllers/subsystem/polling.dm b/code/controllers/subsystem/polling.dm
index 4c3bf452c2c..b237edd3870 100644
--- a/code/controllers/subsystem/polling.dm
+++ b/code/controllers/subsystem/polling.dm
@@ -36,7 +36,7 @@ SUBSYSTEM_DEF(polling)
  * * chat_text_border_icon: Object or path to make an icon of to decorate the chat announcement.
  * * announce_chosen: Whether we should announce the chosen candidates in chat. This is ignored unless amount_to_pick is greater than 0.
  *
- * Returns a list of all mobs who signed up for the poll.
+ * Returns a list of all mobs who signed up for the poll, OR, in the case that amount_to_pick is equal to 1 the singular mob/null if no available candidates.
  */
 /datum/controller/subsystem/polling/proc/poll_candidates(
 	question,
@@ -175,9 +175,11 @@ SUBSYSTEM_DEF(polling)
 	UNTIL(new_poll.finished)
 	if(!(amount_to_pick > 0))
 		return new_poll.signed_up
-	if(length(new_poll.signed_up) < amount_to_pick)
-		return new_poll.signed_up
 	for(var/pick in 1 to amount_to_pick)
+		// There may be less people signed up than amount_to_pick
+		// pick_n_take returns the default return value of null if passed an empty list, so just break in that case rather than adding null to the list.
+		if(!length(new_poll.signed_up))
+			break
 		new_poll.chosen_candidates += pick_n_take(new_poll.signed_up)
 	if(announce_chosen)
 		new_poll.announce_chosen(group)