From deb0615639c8fd1e4b9cee7fdd811d32c3457496 Mon Sep 17 00:00:00 2001
From: Lemongrass3110 <lemongrass@kstp.at>
Date: Thu, 19 Dec 2019 22:12:55 +0100
Subject: [PATCH] Follow up to 2fc7472

Fixed an issue with random calculation on abra in some cases.
Moved some commonly used functions into util and yaml database for global usage.

Thanks to @Daegaladh and @aleos89
---
 src/common/database.cpp  |  8 ++++++++
 src/common/database.hpp  | 13 +++++++++++++
 src/common/utilities.hpp | 14 ++++++++++++++
 src/map/skill.cpp        | 23 ++++++++---------------
 4 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/common/database.cpp b/src/common/database.cpp
index f461aadf96..ea08bf4748 100644
--- a/src/common/database.cpp
+++ b/src/common/database.cpp
@@ -249,6 +249,10 @@ bool YamlDatabase::asUInt16Rate( const YAML::Node& node, const std::string& name
 		if( out > maximum ){
 			this->invalidWarning( node[name], "Node \"%s\" with value %" PRIu16 " exceeds maximum of %" PRIu16 ".\n", name.c_str(), out, maximum );
 
+			return false;
+		}else if( out == 0 ){
+			this->invalidWarning( node[name], "Node \"%s\" needs to be at least 1.\n", name.c_str() );
+
 			return false;
 		}else{
 			return true;
@@ -263,6 +267,10 @@ bool YamlDatabase::asUInt32Rate( const YAML::Node& node, const std::string& name
 		if( out > maximum ){
 			this->invalidWarning( node[name], "Node \"%s\" with value %" PRIu32 " exceeds maximum of %" PRIu32 ".\n", name.c_str(), out, maximum );
 
+			return false;
+		}else if( out == 0 ){
+			this->invalidWarning( node[name], "Node \"%s\" needs to be at least 1.\n", name.c_str() );
+
 			return false;
 		}else{
 			return true;
diff --git a/src/common/database.hpp b/src/common/database.hpp
index db13f00b47..c8ab6b30e3 100644
--- a/src/common/database.hpp
+++ b/src/common/database.hpp
@@ -12,6 +12,7 @@
 
 #include "cbasetypes.hpp"
 #include "core.hpp"
+#include "utilities.hpp"
 
 class YamlDatabase{
 // Internal stuff
@@ -84,6 +85,10 @@ public:
 		this->data.clear();
 	}
 
+	bool empty(){
+		return this->data.empty();
+	}
+
 	bool exists( keytype key ){
 		return this->find( key ) != nullptr;
 	}
@@ -113,6 +118,14 @@ public:
 	size_t size(){
 		return this->data.size();
 	}
+
+	std::shared_ptr<datatype> random(){
+		if( this->empty() ){
+			return nullptr;
+		}
+
+		return rathena::util::umap_random( this->data );
+	}
 };
 
 #endif /* DATABASE_HPP */
diff --git a/src/common/utilities.hpp b/src/common/utilities.hpp
index ea8b9afbd1..70d0200f8e 100644
--- a/src/common/utilities.hpp
+++ b/src/common/utilities.hpp
@@ -10,6 +10,7 @@
 #include <unordered_map>
 
 #include "cbasetypes.hpp"
+#include "random.hpp"
 
 // Class used to perform time measurement
 class cScopeTimer {
@@ -120,6 +121,19 @@ namespace rathena {
 			else
 				return defaultValue;
 		}
+
+		/**
+		 * Get a random value from the given map
+		 * @param map: Unordered Map to search through
+		 * @return A random value by reference
+		*/
+		template <typename K, typename V> V& umap_random( std::unordered_map<K, V>& map ){
+			auto it = map.begin();
+
+			std::advance( it, rnd_value( 0, map.size() - 1 ) );
+
+			return it->second;
+		}
 	}
 }
 
diff --git a/src/map/skill.cpp b/src/map/skill.cpp
index b403a2914e..f50c774132 100755
--- a/src/map/skill.cpp
+++ b/src/map/skill.cpp
@@ -6410,20 +6410,23 @@ int skill_castend_nodamage_id (struct block_list *src, struct block_list *bl, ui
 		break;
 
 	case SA_ABRACADABRA:
-		if (abra_db.size() == 0) {
+		if (abra_db.empty()) {
 			clif_skill_nodamage (src, bl, skill_id, skill_lv, 1);
 			break;
 		}
 		else {
 			int abra_skill_id = 0, abra_skill_lv, checked = 0, checked_max = abra_db.size() * 3;
-			auto abra_spell = abra_db.begin();
 
 			do {
-				std::advance(abra_spell, rnd() % abra_db.size());
+				auto abra_spell = abra_db.random();
 
-				abra_skill_id = abra_spell->second->skill_id;
+				abra_skill_id = abra_spell->skill_id;
 				abra_skill_lv = min(skill_lv, skill_get_max(abra_skill_id));
-			} while (checked++ < checked_max && rnd() % 10000 >= abra_spell->second->per[max(skill_lv - 1, 0)]);
+
+				if( rnd() % 10000 < abra_spell->per[max(skill_lv - 1, 0)] ){
+					break;
+				}
+			} while (checked++ < checked_max);
 
 			if (!skill_get_index(abra_skill_id))
 				break;
@@ -21629,11 +21632,6 @@ uint64 AbraDatabase::parseBodyNode(const YAML::Node &node) {
 			if (!this->asUInt16Rate(probNode, "Probability", probability))
 				return 0;
 
-				if (!probability) {
-					this->invalidWarning(probNode["Probability"], "Probability has to be within the range of 1~10000, skipping.\n");
-					return 0;
-				}
-
 			abra->per.fill(probability);
 		} else {
 			abra->per.fill(0);
@@ -21652,11 +21650,6 @@ uint64 AbraDatabase::parseBodyNode(const YAML::Node &node) {
 				if (!this->asUInt16Rate(it, "Probability", probability))
 					continue;
 
-				if (!probability) {
-					this->invalidWarning(it["Probability"], "Probability has to be within the range of 1~10000, skipping.\n");
-					return 0;
-				}
-
 				abra->per[skill_lv - 1] = probability;
 			}
 		}