TTT: Enable scoreboard to be sorted by different criteria other than score #1332

DirkyJerky · 2017-02-15T02:54:29Z

Reasoning:

Sorting by score isn't very useful.
An unsorted scoreboard is very difficult to use efficiently on large servers
--- It can take several seconds to find a specific person on it, especially if some of them are color coded.
Most people can search an alphabetically sorted list far quicker than one that is not.

Current status:

Titles in the column header row are clickable to make the scoreboard sort by that criteria
TTTScoreboardColumns hook extended so a sorting function can be optionally added for sortable custom columns
Pseudo-markdown pseudo-documentation for said hook

From http://arcadiaservers.com/index.php?threads/sort-names-in-scoreboard-alphabetically.971/#post-7781

bmwalters · 2017-02-15T03:12:10Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+         bt = k
+      end
+   end
+   return bt > at


Couldn't this all be replaced by:

local function CompareRowsByName(rowa, rowb) local plya = rowa:GetPlayer() if not IsValid(plya) then return false end local plyb = rowb:GetPlayer() if not IsValid(plyb) then return true end return plya:GetName() < plyb:GetName() -- might be > end

bmwalters · 2017-02-15T03:13:13Z

garrysmod/gamemodes/terrortown/gamemode/init.lua

@@ -91,6 +94,8 @@ CreateConVar("ttt_det_credits_starting", "1")
 CreateConVar("ttt_det_credits_traitorkill", "0")
 CreateConVar("ttt_det_credits_traitordead", "1")

+-- Other
+CreateConVar("ttt_scoreboard_sorting", "1", FCVAR_NOTIFY + FCVAR_REPLICATED)


Instead of a ConVar, why not allow the clients to sort by clicking on the column headings or something?

Good idea; but I believe I need to keep the ConVar as I could not find a shared object needed to host the variable. There may be one though; I'm not very familiar with the codebase.

It could be a property of the scoreboard object

bmwalters · 2017-02-15T03:13:28Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+
+
+
+


You seem to have quite a bit of whitespace here and above.

bmwalters · 2017-02-15T03:13:48Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+
+local function CompareDeath(pa, pb)
+   if not IsValid(pa) then return false end
+   if not IsValid(pb) then return true end


The rows should always be valid at call time so these checks can be removed.

bmwalters · 2017-02-15T03:14:22Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+   if not IsValid(pb) then return true end
+
+   local a = pa:GetPlayer()
+   local b = pb:GetPlayer()


Might want to use some more descriptive variable names for the rows and players. It took me longer to figure out the purpose of this code than if they had been properly labelled.

Kefta · 2017-02-15T03:16:21Z

A few things: you should use bit.* operations for bit flags instead of normal arithmetic. In this case, CreateConVar actually supports sending in a table of flags that it will merge internally. Next, you should always test pull requests before submitting them; community feedback is great, but it works even better on working code. Also, why are you doing all of that extra sorting work in CompareAlpha?

DirkyJerky · 2017-02-15T03:18:39Z

I just finished testing it; it works fine.

I was not the one that created the initial code; I will work on cleaning it up now

And sorting by clicking on column headers sounds like a fabulous idea.

bmwalters · 2017-02-15T04:52:15Z

@DirkyJerky @Kefta
bmwalters@d8b7007

I implemented clicking on scoreboard headings to sort. It supports swapping the sort direction by clicking a heading multiple times. It's just missing sorting by name.

One quirk is that it sorts within ScoreGroups independently of each other, but it seems fine.

TTT: Implement clicking on scoreboard headings to sort by that field

DirkyJerky · 2017-02-15T05:25:15Z

Great job @bmwalters :D
We could probably make a few fake column labels alongside the other ones to set the name or role sorting options;
Maybe make the text of the labels could change color a bit if it is selected too?
I'll try it tomorrow.

It may even be useful to the end user; probably not

bmwalters · 2017-02-15T14:57:27Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_main.lua

@@ -112,6 +112,23 @@ function PANEL:Init()

   self.ply_groups = {}

+   concommand.Add( "ttt_scoreboard_setsort", function( _, _, args )


i honestly don't think we need this

A ConVar to store how the sorting should be in the scoreboard would be useful, wouldn't it?

markusmarkusz · 2017-02-15T15:45:03Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+         return plya:GetRole() > plyb:GetRole()
+      elseif self.sort_mode == "karma" then
+         return plya:GetBaseKarma() < plyb:GetBaseKarma()
+      else


Maybe add a hook, so you can also sort custom columnes too?

DirkyJerky · 2017-02-15T23:17:45Z

Alright I added two more buttons to click to sort on; and made the active sorting option get highlighted.

DirkyJerky · 2017-02-15T23:25:03Z

Every option is working smoothly it seems.

http://imgur.com/a/HJQMj

DirkyJerky · 2017-02-15T23:33:41Z

In my personal opinion; It would be a much better option to store the current sort option as a ConVar (with ARCHIVE flag set), because then it will be persistent across sessions.

bmwalters · 2017-02-16T01:04:51Z

@DirkyJerky
The extra column background created by the Name and Role headings and the headings themselves don't look very good IMO. I think a good way to sort by name may be clicking on the "Terrorists (10)" heading area of whatever the topmost score group is. We might be able to just forego sorting by role if there isn't a good way to fit it into the design as the row highlighting makes it easy to view roles even without grouping. I'd be interested in the input of others here.

About the ConVar, I don't think the current sort option is something that needs to persist. It's trivial to interact with the headings to change the sorting back when wanted, and it's sort of needless complexity.

Lastly, maybe the green from the "Terrorists (10)" heading background could be re-used for the text color.

Nice job getting some stuff made; it's good to have working prototypes of these features.

I did this once before but reverted it for some reason, I forget why. Works fine it seems

DirkyJerky · 2017-03-10T02:40:51Z

Done.
Thanks for the review!

DirkyJerky · 2017-03-10T03:25:32Z

For the new translation, I was going to scour the internet to see if I could possibly find some existing locale files for the single translation, but I quick checked Google Translate (clichè I know) and through their "community translate" program they seem to have a whole bunch of "verified" translations, one of which is the one I need. Example: https://translate.google.com/#en/pt/Sort%20By%3A

~~Could we use those?~~

Scratch that I found differing translations from different sources, so I bet an actual translator would know what works best.

Kefta · 2017-03-10T03:43:20Z

Don't put in empty translations; let it fall back to English

Kefta · 2017-03-10T04:46:10Z

No, I meant delete the translation from the other files. The code will automatically make it fall back to english.

markusmarkusz · 2017-03-10T16:19:36Z

German Translation:
Sortiere nach:

Kefta · 2017-03-10T16:24:38Z

Spanish is "Ordene por:"

Kefta · 2017-04-17T15:51:38Z

Can you squash?

svdm

Finally got around to testing this, and I did not get very far.

svdm · 2017-04-17T15:56:59Z

garrysmod/gamemodes/terrortown/gamemode/vgui/sb_team.lua

+      local comp = 0 -- Lua doesnt have an Ordering enumeration, I think?
+
+      local sort_mode = GetConVar("ttt_scoreboard_sorting"):GetString()
+      local sort_func = sboard_panel.sort_table[sort_mode]


This use of sboard_panel breaks things. UpdateSortCache is called while initializing the scoreboard. At that point, sboard_panel is nil (because the panel is still being created).

As a result, if 2+ players exist when you first open the scoreboard, then this sboard_panel.sort_table access will of course break and throw your face full of errors.

Note that simply not sorting if sboard_panel == nil probably will not work, because it will just leave the sort cache in an unsorted state until something triggers an update.

Ugh I am bad I should have investigated this more earlier.

I had uncorrectly assumed that because existing code in sb_row.lua and sb_team.lua used that variable without any nil checks, that it was something out of the scope of this PR.

Which I see now was because it was used in instance methods

robotboy655 · 2017-04-17T16:33:05Z

@Kefta, PRs can now be automatically Squashed when merging

Kefta · 2017-04-17T16:54:10Z

Ah, didn't know

DirkyJerky · 2017-04-18T01:31:19Z

I believe the best but messy option is as follows:

Replace sboard_panel with self:GetParent():GetParent():GetParent (nesting dolls are fun)

Goal is the top TTTScoreboard panel, which defines its self.sort_table in its INIT() [----]

Child #1 is TTTPlayerFrame [----]

Child #2 is just a Panel used as a canvas, returned by "TTTPlayerFrame"::GetCanvas() [----]
[----]

(Target) Child #3 is one of the TTTScoreGroups, each of which has the previous Panel as its parent [----]

Everything seems to be working with the fix.

Sorry for disappointing you @svdm ;-;

…ence

svdm · 2017-04-18T10:19:29Z

Honestly it's cleaner to just move the sort_table stuff into a global sboard_sort. It's basically constant anyway. Then you can use that sboard_sort wherever you need it in the scoreboard panels without the nesting mess.

DirkyJerky · 2017-04-18T14:10:52Z

~~Will test at home this afternoon.~~

Seems to still be working as intended.

MinIsMin · 2017-07-27T14:13:42Z

The "Sort by" string is clipping into "Name" in some languages (tested with German and Spanish) and doesn't look well.

Maybe GetPos + space for next position?

DirkyJerky added 5 commits February 14, 2017 17:49

Base changes

a94b8e3

From http://arcadiaservers.com/index.php?threads/sort-names-in-scoreboard-alphabetically.971/#post-7781

Make the convar

c6a6cea

Hook into the convar

c512684

Remove button selector thing; Probably better in a settings menu

6d92af5

Alphabetically by default

53dcb92

bmwalters reviewed Feb 15, 2017

View reviewed changes

DirkyJerky and others added 5 commits February 14, 2017 21:29

Removed unnessesary checks and whitespace

c82a4d0

a,b -> plya, plyb

6af889b

That was a weird CompareAlpha function

4793be8

> -> <

0e74c2a

TTT: Implement clicking on scoreboard headings to sort by that field

d8b7007

DirkyJerky added 3 commits February 14, 2017 23:09

Merge pull request #1 from bmwalters/feature/scoreboard-sorting

9dbeffe

TTT: Implement clicking on scoreboard headings to sort by that field

Well we dont need to ConVar anymore

b01837b

Role option

acad015

Add concommand ttt_scoreboard_setsort

5d59fad

It may even be useful to the end user; probably not

robotboy655 assigned svdm Feb 15, 2017

robotboy655 added the TTT The pull request is for TTT and will be handled by svdm. label Feb 15, 2017

bmwalters reviewed Feb 15, 2017

View reviewed changes

markusmarkusz reviewed Feb 15, 2017

View reviewed changes

DirkyJerky added 2 commits February 15, 2017 17:03

Name & Role headers AND highlight active header

bd5a411

Update color display after sort change

9ffde57

We dont need this concommand anymore

d785f17

DirkyJerky added 4 commits March 9, 2017 20:13

Optimization

e1849a4

I did this once before but reverted it for some reason, I forget why. Works fine it seems

Compare names case insensitively

2ac8002

I remember the issue with the optimization now; I fixed that

b226ea1

Unnessesary underscore

a8d55c9

"Old" New translation

650381f

Default to english via suggestion of @Kefta

dab25a3

Default to english fixed

702e38d

DirkyJerky added 2 commits March 11, 2017 17:04

Translate german via @ markusmarkusz

ff9ab8d

Translate spanish via @ Kefta

7d9a8fe

svdm approved these changes Apr 17, 2017

View reviewed changes

svdm requested changes Apr 17, 2017

View reviewed changes

Use GetParent() x3 instead of refering to the global variable refer…

865151b

…ence

DirkyJerky added 2 commits April 18, 2017 09:09

Global sboard_sort in sb_main.lua

7f51348

Global sboard_sort in sb_team.lua

e5ee199

svdm approved these changes Apr 19, 2017

View reviewed changes

svdm merged commit a71f363 into Facepunch:master Apr 19, 2017

garrynewman unassigned svdm May 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TTT: Enable scoreboard to be sorted by different criteria other than score #1332

TTT: Enable scoreboard to be sorted by different criteria other than score #1332

DirkyJerky commented Feb 15, 2017 •

edited

bmwalters Feb 15, 2017

bmwalters Feb 15, 2017

DirkyJerky Feb 15, 2017

bmwalters Feb 15, 2017

bmwalters Feb 15, 2017

bmwalters Feb 15, 2017

bmwalters Feb 15, 2017

Kefta commented Feb 15, 2017

DirkyJerky commented Feb 15, 2017

bmwalters commented Feb 15, 2017 •

edited

DirkyJerky commented Feb 15, 2017

bmwalters Feb 15, 2017

markusmarkusz Feb 15, 2017 •

edited

markusmarkusz Feb 15, 2017

DirkyJerky commented Feb 15, 2017

DirkyJerky commented Feb 15, 2017 •

edited

DirkyJerky commented Feb 15, 2017

bmwalters commented Feb 16, 2017 •

edited

DirkyJerky commented Mar 10, 2017

DirkyJerky commented Mar 10, 2017 •

edited

Kefta commented Mar 10, 2017

Kefta commented Mar 10, 2017

markusmarkusz commented Mar 10, 2017

Kefta commented Mar 10, 2017

Kefta commented Apr 17, 2017

svdm left a comment

svdm Apr 17, 2017

DirkyJerky Apr 18, 2017 •

edited

robotboy655 commented Apr 17, 2017

Kefta commented Apr 17, 2017

DirkyJerky commented Apr 18, 2017 •

edited

svdm commented Apr 18, 2017

DirkyJerky commented Apr 18, 2017 •

edited

MinIsMin commented Jul 27, 2017 •

edited

		@@ -112,6 +112,23 @@ function PANEL:Init()

		self.ply_groups = {}

		concommand.Add( "ttt_scoreboard_setsort", function( _, _, args )

TTT: Enable scoreboard to be sorted by different criteria other than score #1332

TTT: Enable scoreboard to be sorted by different criteria other than score #1332

Conversation

DirkyJerky commented Feb 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kefta commented Feb 15, 2017

DirkyJerky commented Feb 15, 2017

bmwalters commented Feb 15, 2017 • edited

DirkyJerky commented Feb 15, 2017

Choose a reason for hiding this comment

markusmarkusz Feb 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DirkyJerky commented Feb 15, 2017

DirkyJerky commented Feb 15, 2017 • edited

DirkyJerky commented Feb 15, 2017

bmwalters commented Feb 16, 2017 • edited

DirkyJerky commented Mar 10, 2017

DirkyJerky commented Mar 10, 2017 • edited

Kefta commented Mar 10, 2017

Kefta commented Mar 10, 2017

markusmarkusz commented Mar 10, 2017

Kefta commented Mar 10, 2017

Kefta commented Apr 17, 2017

svdm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DirkyJerky Apr 18, 2017 • edited

Choose a reason for hiding this comment

robotboy655 commented Apr 17, 2017

Kefta commented Apr 17, 2017

DirkyJerky commented Apr 18, 2017 • edited

svdm commented Apr 18, 2017

DirkyJerky commented Apr 18, 2017 • edited

MinIsMin commented Jul 27, 2017 • edited

DirkyJerky commented Feb 15, 2017 •

edited

bmwalters commented Feb 15, 2017 •

edited

markusmarkusz Feb 15, 2017 •

edited

DirkyJerky commented Feb 15, 2017 •

edited

bmwalters commented Feb 16, 2017 •

edited

DirkyJerky commented Mar 10, 2017 •

edited

DirkyJerky Apr 18, 2017 •

edited

DirkyJerky commented Apr 18, 2017 •

edited

DirkyJerky commented Apr 18, 2017 •

edited

MinIsMin commented Jul 27, 2017 •

edited