Add player count to scoreboard #274

GooberRF · 2024-09-16T18:36:58Z

Adds the current server player count to the scoreboard for all gamemodes as shown in the screenshots below.

Resolves #27

is-this-c · 2024-09-17T02:47:47Z

I do not really like its verbosity. I think it should have a console command.

GooberRF · 2024-09-17T03:21:57Z

I do not really like its verbosity.

I don't think it's verbose at all. I've also tested with bighud on and off, all gamemodes - in no case is it even close to in danger of getting cut off. IMO this is a straight improvement over the original.

I think it should have a console command.

I'm not opposed to it being associated with a console command as toggle, but I also don't think it's necessary. Will wait to hear rafalh's take on this point - I'd be happy to tie it to a console command if he wants it that way.

is-this-c · 2024-09-17T08:33:15Z

It looks much better like this imo. (My scoreboard is slightly altered.)

GooberRF · 2024-09-17T13:25:30Z

It looks much better like this imo.

I like that, but I don't think removing the "Red Faction" logo is a good idea, at least as a default. I do like the use of a pipe as opposed to the " WITH ", though:

is-this-c · 2024-09-17T14:16:26Z

but I don't think removing the "Red Faction" logo is a good idea

When I mentioned my scoreboard is altered (it is old code) it was about this ugly logo 😊. Moreover I tested a few variations and - was good but | probably better.

rafalh · 2024-11-16T01:56:18Z

game_patch/hud/multi_scoreboard.cpp

-            game_type_name = rf::strings::team_deathmatch;
-        rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_type_name);
+        int num_players = rf::multi_num_players();
+        std::string player_count_str =


could number of heap allocations be reduced? game_type_name don't have to be an std::string, it could be const char* or std::string_view. The string used for concatenation could have reserved memory before concatenation.
This code runs every frame so it should be fast and allocations are generally considered slow and can lead to micro-stutters. Also please consider using std::format_to

rafalh · 2024-11-16T01:59:33Z

game_patch/hud/multi_scoreboard.cpp

+        std::string player_count_str =
+            " | " + std::to_string(num_players) + (num_players > 1 ? " PLAYERS" : " PLAYER");
+
+        std::string game_type_name = (game_type == rf::NG_TYPE_DM)    ? rf::strings::deathmatch


I don't really like double ternary operator. Why did you change it? What if we added a forth game mode? It would look terrible.
Not to mention formatting is weird.

is-this-c · 2024-11-30T17:32:07Z

game_patch/hud/multi_scoreboard.cpp

+        int num_players = rf::multi_num_players();
+        std::string player_count_str =
+            " | " + std::to_string(num_players) + (num_players > 1 ? " PLAYERS" : " PLAYER");
+
+        std::string game_type_name = (game_type == rf::NG_TYPE_DM)    ? rf::strings::deathmatch
+                                     : (game_type == rf::NG_TYPE_CTF) ? rf::strings::capture_the_flag
+                                                                      : rf::strings::team_deathmatch;
+
+        game_type_name += player_count_str;
+
+        rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_type_name.c_str());


Well I completely rewrote everything. It now ignores browsers. 1 allocation.

Suggested change

int num_players = rf::multi_num_players();

std::string player_count_str =

" | " + std::to_string(num_players) + (num_players > 1 ? " PLAYERS" : " PLAYER");

std::string game_type_name = (game_type == rf::NG_TYPE_DM) ? rf::strings::deathmatch

: (game_type == rf::NG_TYPE_CTF) ? rf::strings::capture_the_flag

: rf::strings::team_deathmatch;

game_type_name += player_count_str;

rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_type_name.c_str());

const std::string_view game_type_name = std::invoke([game_type] {

if (game_type == rf::NG_TYPE_DM) {

return rf::strings::deathmatch;

} else if (game_type == rf::NG_TYPE_CTF) {

return rf::strings::capture_the_flag;

} else {

return rf::strings::team_deathmatch;

}

});

int32_t num_players = 0;

for (rf::Player& player : SinglyLinkedList{rf::player_list}) {

if (!get_player_additional_data(&player).is_browser) {

++num_players;

}

}

const std::string game_info = std::format(

"{} | {} {}",

game_type_name,

num_players,

num_players > 1 ? "PLAYERS" : "PLAYER"

);

rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_info.c_str());

I'm not opposed to the rewrite, I am opposed to excluding browsers from this calculation, though, at least as long as they appear on the scoreboard without any clear identifier.

Having the count at the top of the scoreboard less than the number of names shown as players on the scoreboard would be very confusing.

#306 paves the way for introducing an identifier for browser clients. Let's see what rafalh says.

I totally agree with Goober that without a clear indication what we count it will be confusing to skip browsers.
Still, even if we add some image besides browser clients it will still be a bit confusing if people see a different count in server list screen and after joining the server. Maybe we should display two counters: number of all players, and number of browsers or add some small hint that the counter does not include browsers?
Anyway why we want to skip browsers? If we care about players actively playing the game AFK players could also be treated differently?
I think we should prevent scope-creep in this PR and leave the browsers issue to another one

About the proposed code, it looks ok, but I would prefer a standalone function over std::invoke with a lambda here

An idea could be to display a number of player slots occupied and separately, a number of currently spawned players?

I don't know how to appropriately communicate that on the UI though - there's not a lot of room to work with.

Agreed though that scope creep should be prevented here. Anything beyond displaying a number of occupied player slots should be out of scope for this simple change IMO.

Perhaps it could be num_spawned_players/num_players PLAYING e.g. 5/8 PLAYING.

sounds good, although I'm not sure if counting alive people "as is" is the best approach because people spawn and die all the time...
to be honest i'm not sure if clobbering panel with multiple statistics is worth it considering it can reduce readability. For me it's okay to just display total player count which in most cases is enough, but if you add "playing" I will accept that too

GooberRF added 2 commits September 16, 2024 16:03

Add player count to scoreboard in all gamemodes

1e0a84a

Merge branch 'rafalh:master' into playercountscoreboard

2fefeb4

GooberRF added 2 commits September 17, 2024 10:59

replace "WITH" with pipe

d1deb74

Merge branch 'rafalh:master' into playercountscoreboard

a7bc671

GooberRF and others added 2 commits September 24, 2024 14:43

Merge branch 'rafalh:master' into playercountscoreboard

be8a631

Merge branch 'master' into playercountscoreboard

9119377

rafalh requested changes Nov 16, 2024

View reviewed changes

is-this-c reviewed Nov 30, 2024

View reviewed changes

is-this-c mentioned this pull request Dec 8, 2024

Add a spawned player count to the scoreboard #316

Merged

GooberRF closed this Feb 3, 2025

GooberRF deleted the playercountscoreboard branch February 3, 2025 18:52

-        int num_players = rf::multi_num_players();
-        std::string player_count_str =
-            " | " + std::to_string(num_players) + (num_players > 1 ? " PLAYERS" : " PLAYER");
-        std::string game_type_name = (game_type == rf::NG_TYPE_DM)    ? rf::strings::deathmatch
-                                     : (game_type == rf::NG_TYPE_CTF) ? rf::strings::capture_the_flag
-                                                                      : rf::strings::team_deathmatch;
-        game_type_name += player_count_str;
-        rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_type_name.c_str());
+        const std::string_view game_type_name = std::invoke([game_type] {
+            if (game_type == rf::NG_TYPE_DM) {
+                return rf::strings::deathmatch;
+            } else if (game_type == rf::NG_TYPE_CTF) {
+                return rf::strings::capture_the_flag;
+            } else {
+                return rf::strings::team_deathmatch;
+            }
+        });
+        int32_t num_players = 0;
+        for (rf::Player& player : SinglyLinkedList{rf::player_list}) {
+            if (!get_player_additional_data(&player).is_browser) {
+                ++num_players;
+            }
+        }
+        const std::string game_info = std::format(
+            "{} | {} {}",
+            game_type_name,
+            num_players,
+            num_players > 1 ? "PLAYERS" : "PLAYER"
+        );
+        rf::gr::string_aligned(rf::gr::ALIGN_CENTER, x_center, cur_y, game_info.c_str());

Add player count to scoreboard #274

Add player count to scoreboard #274

Conversation

GooberRF commented Sep 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

is-this-c commented Sep 17, 2024

Uh oh!

GooberRF commented Sep 17, 2024

Uh oh!

is-this-c commented Sep 17, 2024

Uh oh!

GooberRF commented Sep 17, 2024

Uh oh!

is-this-c commented Sep 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

is-this-c Nov 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

3 participants

GooberRF commented Sep 16, 2024 •

edited

Loading

is-this-c Nov 30, 2024 •

edited

Loading