From e766f921f6625febca526f39bfcfbe4a6dd7da58 Mon Sep 17 00:00:00 2001 From: Jeremiah <42397676+jlsnow301@users.noreply.github.com> Date: Sat, 4 May 2024 20:33:52 -0700 Subject: [PATCH] Code cleanup: Sorting (#83017) ## About The Pull Request 1. Removes code duplication 2. Fully documents `sortTim()` 3. Makes a define with default sortTim behavior, straight and to the point for 95% of cases 4. Migrates other sorts into the same file 5. Removes some redundancy where they're reassigning a variable using an in place sorter For the record, we only use timSort ## Why It's Good For The Game More documentation, easier to read, uses `length` over `len`, etc Should be no gameplay effect at all --- code/__HELPERS/hallucinations.dm | 2 +- code/__HELPERS/sorts/InsertSort.dm | 19 ---- code/__HELPERS/sorts/MergeSort.dm | 19 ---- code/__HELPERS/sorts/TimSort.dm | 20 ---- code/__HELPERS/sorts/helpers.dm | 95 +++++++++++++++++++ .../sorts/{__main.dm => sort_instance.dm} | 0 code/controllers/subsystem/dcs.dm | 2 +- code/datums/datum.dm | 2 +- code/modules/admin/verbs/debug.dm | 2 +- code/modules/admin/verbs/light_debug.dm | 2 +- .../malf_ai/malf_ai_module_picker.dm | 2 +- code/modules/antagonists/pirate/pirate.dm | 2 +- code/modules/asset_cache/assets/uplink.dm | 2 +- .../file_system/programs/emojipedia.dm | 2 +- code/modules/research/stock_parts.dm | 2 +- code/modules/unit_tests/unit_test.dm | 2 +- tgstation.dme | 6 +- 17 files changed, 108 insertions(+), 73 deletions(-) delete mode 100644 code/__HELPERS/sorts/InsertSort.dm delete mode 100644 code/__HELPERS/sorts/MergeSort.dm delete mode 100644 code/__HELPERS/sorts/TimSort.dm create mode 100644 code/__HELPERS/sorts/helpers.dm rename code/__HELPERS/sorts/{__main.dm => sort_instance.dm} (100%) diff --git a/code/__HELPERS/hallucinations.dm b/code/__HELPERS/hallucinations.dm index 1eeab08d876..edd65ee926a 100644 --- a/code/__HELPERS/hallucinations.dm +++ b/code/__HELPERS/hallucinations.dm @@ -145,7 +145,7 @@ ADMIN_VERB(debug_hallucination_weighted_list_per_type, R_DEBUG, "Show Hallucinat last_type_weight = this_weight // Sort by weight descending, where weight is the values (not the keys). We assoc_to_keys later to get JUST the text - all_weights = sortTim(all_weights, GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE) + sortTim(all_weights, GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE) var/page_style = "" var/page_contents = "[page_style][header][jointext(assoc_to_keys(all_weights), "")]
" diff --git a/code/__HELPERS/sorts/InsertSort.dm b/code/__HELPERS/sorts/InsertSort.dm deleted file mode 100644 index 2d5ca408ce3..00000000000 --- a/code/__HELPERS/sorts/InsertSort.dm +++ /dev/null @@ -1,19 +0,0 @@ -//simple insertion sort - generally faster than merge for runs of 7 or smaller -/proc/sortInsert(list/L, cmp=/proc/cmp_numeric_asc, associative, fromIndex=1, toIndex=0) - if(L && L.len >= 2) - fromIndex = fromIndex % L.len - toIndex = toIndex % (L.len+1) - if(fromIndex <= 0) - fromIndex += L.len - if(toIndex <= 0) - toIndex += L.len + 1 - - var/datum/sort_instance/SI = GLOB.sortInstance - if(!SI) - SI = new - SI.L = L - SI.cmp = cmp - SI.associative = associative - - SI.binarySort(fromIndex, toIndex, fromIndex) - return L diff --git a/code/__HELPERS/sorts/MergeSort.dm b/code/__HELPERS/sorts/MergeSort.dm deleted file mode 100644 index 4692534edff..00000000000 --- a/code/__HELPERS/sorts/MergeSort.dm +++ /dev/null @@ -1,19 +0,0 @@ -//merge-sort - gernerally faster than insert sort, for runs of 7 or larger -/proc/sortMerge(list/L, cmp=/proc/cmp_numeric_asc, associative, fromIndex=1, toIndex) - if(L && L.len >= 2) - fromIndex = fromIndex % L.len - toIndex = toIndex % (L.len+1) - if(fromIndex <= 0) - fromIndex += L.len - if(toIndex <= 0) - toIndex += L.len + 1 - - var/datum/sort_instance/SI = GLOB.sortInstance - if(!SI) - SI = new - SI.L = L - SI.cmp = cmp - SI.associative = associative - - SI.mergeSort(fromIndex, toIndex) - return L diff --git a/code/__HELPERS/sorts/TimSort.dm b/code/__HELPERS/sorts/TimSort.dm deleted file mode 100644 index 44f6d170df4..00000000000 --- a/code/__HELPERS/sorts/TimSort.dm +++ /dev/null @@ -1,20 +0,0 @@ -//TimSort interface -/proc/sortTim(list/L, cmp=/proc/cmp_numeric_asc, associative, fromIndex=1, toIndex=0) - if(L && L.len >= 2) - fromIndex = fromIndex % L.len - toIndex = toIndex % (L.len+1) - if(fromIndex <= 0) - fromIndex += L.len - if(toIndex <= 0) - toIndex += L.len + 1 - - var/datum/sort_instance/SI = GLOB.sortInstance - if(!SI) - SI = new - - SI.L = L - SI.cmp = cmp - SI.associative = associative - - SI.timSort(fromIndex, toIndex) - return L diff --git a/code/__HELPERS/sorts/helpers.dm b/code/__HELPERS/sorts/helpers.dm new file mode 100644 index 00000000000..7198286f29f --- /dev/null +++ b/code/__HELPERS/sorts/helpers.dm @@ -0,0 +1,95 @@ +/// Sorts the list in place with timSort, default settings. +#define SORT_TIM(to_sort, associative) if(length(to_sort) >= 2) { \ + var/datum/sort_instance/sorter = GLOB.sortInstance; \ + if (isnull(sorter)) { \ + sorter = new; \ + } \ + sorter.L = to_sort; \ + sorter.cmp = GLOBAL_PROC_REF(cmp_numeric_asc); \ + sorter.associative = associative; \ + sorter.timSort(1, 0); \ +} + + +/// Helper for the sorting procs. Prevents some code duplication. Creates /datum/sort_instance/sorter +#define CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex) \ + if(length(to_sort) < 2) { \ + return to_sort; \ + } \ + fromIndex = fromIndex % length(to_sort); \ + toIndex = toIndex % (length(to_sort) + 1); \ + if (fromIndex <= 0) { \ + fromIndex += length(to_sort); \ + } \ + if (toIndex <= 0) { \ + toIndex += length(to_sort) + 1; \ + } \ + var/datum/sort_instance/sorter = GLOB.sortInstance; \ + if (isnull(sorter)) { \ + sorter = new; \ + } \ + sorter.L = to_sort; \ + sorter.cmp = cmp; \ + sorter.associative = associative; + + +/** + * ## Tim Sort + * Hybrid sorting algorithm derived from merge sort and insertion sort. + * + * **Sorts in place**. + * You might not need to get the return value. + * + * @see + * https://en.wikipedia.org/wiki/Timsort + * + * @param {list} to_sort - The list to sort. + * + * @param {proc} cmp - The comparison proc to use. Default: Numeric ascending. + * + * @param {boolean} associative - Whether the list is associative. Default: FALSE. + * + * @param {int} fromIndex - The index to start sorting from. Default: 1. + * + * @param {int} toIndex - The index to stop sorting at. Default: 0. + */ +/proc/sortTim(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list + CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex) + + sorter.timSort(fromIndex, toIndex) + + return to_sort + + +/** + * ## Merge Sort + * Divide and conquer sorting algorithm. + * + * @see + * - https://en.wikipedia.org/wiki/Merge_sort + */ +/proc/sortMerge(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list + CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex) + + sorter.mergeSort(fromIndex, toIndex) + + return to_sort + + +/** + * ## Insertion Sort + * Simple sorting algorithm that builds the final sorted list one item at a time. + * + + * @see + * - https://en.wikipedia.org/wiki/Insertion_sort + */ +/proc/sortInsert(list/to_sort, cmp = GLOBAL_PROC_REF(cmp_numeric_asc), associative = FALSE, fromIndex = 1, toIndex = 0) as /list + CREATE_SORT_INSTANCE(to_sort, cmp, associative, fromIndex, toIndex) + + sorter.binarySort(fromIndex, toIndex) + + return to_sort + + +#undef CREATE_SORT_INSTANCE diff --git a/code/__HELPERS/sorts/__main.dm b/code/__HELPERS/sorts/sort_instance.dm similarity index 100% rename from code/__HELPERS/sorts/__main.dm rename to code/__HELPERS/sorts/sort_instance.dm diff --git a/code/controllers/subsystem/dcs.dm b/code/controllers/subsystem/dcs.dm index eefbede3087..a3dcc26af54 100644 --- a/code/controllers/subsystem/dcs.dm +++ b/code/controllers/subsystem/dcs.dm @@ -84,7 +84,7 @@ PROCESSING_SUBSYSTEM_DEF(dcs) fullid += REF(key) if(named_arguments) - named_arguments = sortTim(named_arguments, GLOBAL_PROC_REF(cmp_text_asc)) + sortTim(named_arguments, GLOBAL_PROC_REF(cmp_text_asc)) fullid += named_arguments return list2params(fullid) diff --git a/code/datums/datum.dm b/code/datums/datum.dm index 77d787eb85b..6288244667e 100644 --- a/code/datums/datum.dm +++ b/code/datums/datum.dm @@ -328,7 +328,7 @@ ASSERT(isatom(src) || isimage(src)) var/atom/atom_cast = src // filters only work with images or atoms. atom_cast.filters = null - filter_data = sortTim(filter_data, GLOBAL_PROC_REF(cmp_filter_data_priority), TRUE) + sortTim(filter_data, GLOBAL_PROC_REF(cmp_filter_data_priority), TRUE) for(var/filter_raw in filter_data) var/list/data = filter_data[filter_raw] var/list/arguments = data.Copy() diff --git a/code/modules/admin/verbs/debug.dm b/code/modules/admin/verbs/debug.dm index e56eb1e5101..20cdf351459 100644 --- a/code/modules/admin/verbs/debug.dm +++ b/code/modules/admin/verbs/debug.dm @@ -765,7 +765,7 @@ ADMIN_VERB(reestablish_tts_connection, R_DEBUG, "Re-establish Connection To TTS" var/list/sorted = list() for (var/source in per_source) sorted += list(list("source" = source, "count" = per_source[source])) - sorted = sortTim(sorted, GLOBAL_PROC_REF(cmp_timer_data)) + sortTim(sorted, GLOBAL_PROC_REF(cmp_timer_data)) // Now that everything is sorted, compile them into an HTML output var/output = "" diff --git a/code/modules/admin/verbs/light_debug.dm b/code/modules/admin/verbs/light_debug.dm index ae5b24ff380..eac81f6ed29 100644 --- a/code/modules/admin/verbs/light_debug.dm +++ b/code/modules/admin/verbs/light_debug.dm @@ -10,7 +10,7 @@ sum[source.source_atom.type] += 1 total += 1 - sum = sortTim(sum, /proc/cmp_numeric_asc, TRUE) + sortTim(sum, associative = TRUE) var/text = "" for(var/type in sum) text += "[type] = [sum[type]]\n" diff --git a/code/modules/antagonists/malf_ai/malf_ai_module_picker.dm b/code/modules/antagonists/malf_ai/malf_ai_module_picker.dm index 51058f82ac0..0ac27c14c97 100644 --- a/code/modules/antagonists/malf_ai/malf_ai_module_picker.dm +++ b/code/modules/antagonists/malf_ai/malf_ai_module_picker.dm @@ -24,7 +24,7 @@ filtered_modules[AM.category][AM] = AM for(var/category in filtered_modules) - filtered_modules[category] = sortTim(filtered_modules[category], GLOBAL_PROC_REF(cmp_malfmodules_priority)) + sortTim(filtered_modules[category], GLOBAL_PROC_REF(cmp_malfmodules_priority)) return filtered_modules diff --git a/code/modules/antagonists/pirate/pirate.dm b/code/modules/antagonists/pirate/pirate.dm index 15a028b24d7..0fa80f55247 100644 --- a/code/modules/antagonists/pirate/pirate.dm +++ b/code/modules/antagonists/pirate/pirate.dm @@ -85,7 +85,7 @@ //Lists notable loot. if(!cargo_hold || !cargo_hold.total_report) return "Nothing" - cargo_hold.total_report.total_value = sortTim(cargo_hold.total_report.total_value, cmp = GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE) + sortTim(cargo_hold.total_report.total_value, cmp = GLOBAL_PROC_REF(cmp_numeric_dsc), associative = TRUE) var/count = 0 var/list/loot_texts = list() for(var/datum/export/E in cargo_hold.total_report.total_value) diff --git a/code/modules/asset_cache/assets/uplink.dm b/code/modules/asset_cache/assets/uplink.dm index e283b86c042..e85ee1b35b5 100644 --- a/code/modules/asset_cache/assets/uplink.dm +++ b/code/modules/asset_cache/assets/uplink.dm @@ -9,7 +9,7 @@ var/list/items = list() for(var/datum/uplink_category/category as anything in subtypesof(/datum/uplink_category)) categories += category - categories = sortTim(categories, GLOBAL_PROC_REF(cmp_uplink_category_desc)) + sortTim(categories, GLOBAL_PROC_REF(cmp_uplink_category_desc)) var/list/new_categories = list() for(var/datum/uplink_category/category as anything in categories) diff --git a/code/modules/modular_computers/file_system/programs/emojipedia.dm b/code/modules/modular_computers/file_system/programs/emojipedia.dm index 6e9bf56a381..10be69cc349 100644 --- a/code/modules/modular_computers/file_system/programs/emojipedia.dm +++ b/code/modules/modular_computers/file_system/programs/emojipedia.dm @@ -14,7 +14,7 @@ /datum/computer_file/program/emojipedia/New() . = ..() // Sort the emoji list so it's easier to find things and we don't have to keep sorting on ui_data since the number of emojis can not change in-game. - emoji_list = sortTim(emoji_list, /proc/cmp_text_asc) + sortTim(emoji_list, /proc/cmp_text_asc) /datum/computer_file/program/emojipedia/ui_static_data(mob_user) var/list/data = list() diff --git a/code/modules/research/stock_parts.dm b/code/modules/research/stock_parts.dm index 79d24b9c55a..d4cb4afefda 100644 --- a/code/modules/research/stock_parts.dm +++ b/code/modules/research/stock_parts.dm @@ -207,7 +207,7 @@ If you create T5+ please take a pass at mech_fabricator.dm. The parts being good continue part_list += component_part //Sort the parts. This ensures that higher tier items are applied first. - part_list = sortTim(part_list, GLOBAL_PROC_REF(cmp_rped_sort)) + sortTim(part_list, GLOBAL_PROC_REF(cmp_rped_sort)) return part_list /proc/cmp_rped_sort(obj/item/first_item, obj/item/second_item) diff --git a/code/modules/unit_tests/unit_test.dm b/code/modules/unit_tests/unit_test.dm index 332005070d5..9a6f89fddc3 100644 --- a/code/modules/unit_tests/unit_test.dm +++ b/code/modules/unit_tests/unit_test.dm @@ -340,7 +340,7 @@ GLOBAL_VAR_INIT(focused_tests, focused_tests()) if(length(focused_tests)) tests_to_run = focused_tests - tests_to_run = sortTim(tests_to_run, GLOBAL_PROC_REF(cmp_unit_test_priority)) + sortTim(tests_to_run, GLOBAL_PROC_REF(cmp_unit_test_priority)) var/list/test_results = list() diff --git a/tgstation.dme b/tgstation.dme index c7cea8aae3a..997e0fb5105 100644 --- a/tgstation.dme +++ b/tgstation.dme @@ -504,10 +504,8 @@ #include "code\__HELPERS\paths\jps.dm" #include "code\__HELPERS\paths\path.dm" #include "code\__HELPERS\paths\sssp.dm" -#include "code\__HELPERS\sorts\__main.dm" -#include "code\__HELPERS\sorts\InsertSort.dm" -#include "code\__HELPERS\sorts\MergeSort.dm" -#include "code\__HELPERS\sorts\TimSort.dm" +#include "code\__HELPERS\sorts\helpers.dm" +#include "code\__HELPERS\sorts\sort_instance.dm" #include "code\_globalvars\_regexes.dm" #include "code\_globalvars\admin.dm" #include "code\_globalvars\arcade.dm"