COMMITTED: More detailed merchant rejection code

[noudess]

The code (at least the version I have from eqemu) that tells a PC to go pound salt, isn't good enough to avoid situations where a half elf is told by a half elf that his race should go elsewhere.

This patch changes it so the merchant decides what to say based on the worst contributor vs primary faction be it class, race, deity or nasty deeds.

Use it if you like, seems to be working as intended on my server.

=== modified file 'zone/client.h'
--- zone/client.h	2014-07-25 14:55:13 +0000
+++ zone/client.h	2014-08-27 19:58:49 +0000
@@ -577,7 +577,7 @@
     FACTION_VALUE   GetFactionLevel(uint32 char_id, uint32 npc_id, uint32 p_race, uint32 p_class, uint32 p_deity, int32 pFaction, Mob* tnpc);
 	int32	GetCharacterFactionLevel(int32 faction_id);
 	int32  GetModCharacterFactionLevel(int32 faction_id);
-	bool	HatedByClass(uint32 p_race, uint32 p_class, uint32 p_deity, int32 pFaction);
+	string	WhyHated(uint32 p_race, uint32 p_class, uint32 p_deity, int32 pFaction);
 
 	void	SetFactionLevel(uint32 char_id, uint32 npc_id, uint8 char_class, uint8 char_race, uint8 char_deity);
 	void    SetFactionLevel2(uint32 char_id, int32 faction_id, uint8 char_class, uint8 char_race, uint8 char_deity, int32 value, uint8 temp);

=== modified file 'zone/client_packet.cpp'
--- zone/client_packet.cpp	2014-08-15 18:12:09 +0000
+++ zone/client_packet.cpp	2014-08-27 20:02:52 +0000
@@ -5465,25 +5465,21 @@
 	int factionlvl = GetFactionLevel(CharacterID(), tmp->CastToNPC()->GetNPCTypeID(), GetRace(), GetClass(), GetDeity(), tmp->CastToNPC()->GetPrimaryFaction(), tmp);
 	if(factionlvl >= 7)
 	{
-		char playerp[16] = "players";
-		if(HatedByClass(GetRace(), GetClass(), GetDeity(), tmp->CastToNPC()->GetPrimaryFaction()))
-			strcpy(playerp,GetClassPlural(this));
-		else
-			strcpy(playerp,GetRacePlural(this));
+		string playerp = WhyHated(GetRace(), GetClass(), GetDeity(), tmp->CastToNPC()->GetPrimaryFaction());
 
 		uint8 rand_ = rand() % 4;
 		switch(rand_){
 			case 1:
-				Message(0,"%s says 'It's not enough that you %s have ruined your own lands. Now get lost!'", tmp->GetCleanName(), playerp);
+				Message(0,"%s says 'It's not enough that you %s have ruined your own lands. Now get lost!'", tmp->GetCleanName(), playerp.data());
 				break;
 			case 2:
-				Message(0,"%s says 'I have something here that %s use... let me see... it's the EXIT, now get LOST!'", tmp->GetCleanName(), playerp);
+				Message(0,"%s says 'I have something here that %s use... let me see... it's the EXIT, now get LOST!'", tmp->GetCleanName(), playerp.data());
 				break;
 			case 3:
-				Message(0,"%s says 'Don't you %s have your own merchants? Whatever, I'm not selling anything to you!'", tmp->GetCleanName(), playerp);
+				Message(0,"%s says 'Don't you %s have your own merchants? Whatever, I'm not selling anything to you!'", tmp->GetCleanName(), playerp.data());
 				break;
 			default:
-				Message(0,"%s says 'I don't like to speak to %s much less sell to them!'", tmp->GetCleanName(), playerp);
+				Message(0,"%s says 'I don't like to speak to %s much less sell to them!'", tmp->GetCleanName(), playerp.data());
 				break;
 		}
 		action = 0;

=== modified file 'zone/faction.cpp'
--- zone/faction.cpp	2013-07-09 15:28:09 +0000
+++ zone/faction.cpp	2014-08-27 20:09:18 +0000
@@ -1037,28 +1037,38 @@
 	return true;
 }
 
-bool Client::HatedByClass(uint32 p_race, uint32 p_class, uint32 p_deity, int32 pFaction)
+string Client::WhyHated(uint32 p_race, uint32 p_class, uint32 p_deity, int32 pFaction)
 {
-	
-	bool Result = false;
+	string result="defilers of our beliefs"
 	_ZP(Client_GetFactionLevel);
 
 	int32 tmpFactionValue;
 	FactionMods fmods;
+	int32 lowestvalue;
 
-    //First get the NPC's Primary faction
+	//First get the NPC's Primary faction
 	if(pFaction > 0)
-	{
+		{
 		//Get the faction data from the database
 		if(database.GetFactionData(&fmods, p_class, p_race, p_deity, pFaction))
-		{
+			{
 			tmpFactionValue = GetCharacterFactionLevel(pFaction);
-			tmpFactionValue += GetFactionBonus(pFaction);
-			tmpFactionValue += GetItemFactionBonus(pFaction);
-			CalculateFaction(&fmods, tmpFactionValue);
-			if(fmods.class_mod < fmods.race_mod)
-				Result = true;
+			lowestvalue=min(tmpFactionValue, min(fmods.class_mod, 
+								min(fmods.race_mod, fmods.deity_mod)));
+			if (lowestvalue == fmods.class_mod)
+				{
+				result=GetClassPlural(this);
+				}
+			else if (lowestvalue == fmods.race_mod)
+				{
+				result=GetRacePlural(this);;
+				}
+			else if (lowestvalue == fmods.deity_mod)
+				{
+				result="worshippers of " + GetDeityName(p_deity);
+				}
+			}
 		}
-	}
-	return Result;
+
+	return result;
 }

=== modified file 'zone/mob.cpp'
--- zone/mob.cpp	2014-07-22 19:15:11 +0000
+++ zone/mob.cpp	2014-08-27 20:13:08 +0000
@@ -30,6 +30,7 @@
 #include "../common/rulesys.h"
 #include "../common/emu_opcodes.h"
 #include "../common/eq_packet_structs.h"
+#include "../common/deity.h"
 #include "zonedb.h"
 #include "../common/packet_dump.h"
 #include "../common/packet_functions.h"
@@ -4598,3 +4599,31 @@
 }
 
 
+string Mob::GetDeityName(uint8 deity)
+
+{
+	string name;
+
+	switch (deity)
+		{
+		case DEITY_AGNOSTIC:	name="AGNOSTIC"; break;
+		case DEITY_BRELL:		name="BRELL"; break;
+		case DEITY_CAZIC:		name="CAZIC"; break;
+		case DEITY_EROLLSI:		name="EROLLSI"; break;
+		case DEITY_BRISTLE:		name="BRISTLE"; break;
+		case DEITY_INNY:		name="INNY"; break;
+		case DEITY_KARANA:		name="KARANA"; break;
+		case DEITY_MITH:		name="MITH"; break;
+		case DEITY_PREXUS:		name="PREXUS"; break;
+		case DEITY_QUELLIOUS:	name="QUELLIOUS"; break;
+		case DEITY_RALLOS:		name="RALLOS"; break;
+		case DEITY_SOLUSEK:		name="SOLUSEK"; break;
+		case DEITY_TRIBUNAL:	name="TRIBUNAL"; break;
+		case DEITY_TUNARE:		name="TUNARE"; break;
+		case DEITY_BERTOX:		name="BERTOX"; break;
+		case DEITY_RODCET:		name="RODCET"; break;
+		case DEITY_VEESHAN:		name="VEESHAN"; break;
+		} 
+	
+	return name;
+}

=== modified file 'zone/mob.h'
--- zone/mob.h	2014-07-22 19:15:11 +0000
+++ zone/mob.h	2014-08-27 19:53:40 +0000
@@ -732,6 +732,7 @@
 	virtual void SetTarget(Mob* mob);
 	virtual inline float		GetHPRatio() const { return max_hp == 0 ? 0 : ((float)cur_hp/max_hp*100); }
 
+	string GetDeityName(uint8 deity);
 	bool IsLoggingEnabled() const { return(logging_enabled); }
 	void EnableLogging() { logging_enabled = true; }
 	void DisableLogging() { logging_enabled = false; }

[Secrets]

You should really change this to use the eqstr_us.txt versions.

1154 I don't like to speak to %B3(12) much less sell to them!
1155 It's %B3(13) like you that are ruining the continent...get OUT!
1156 Isn't there some kind of ordinance against %B3(13) crawling out from under their rocks?
1157 %B3(13) like you don't have any place in my shop..now make way for welcome customers.
1158 I thought scumbag %B3(13) like you just stole whatever they need.  Now GET OUT!
1159 I don't have anything to do with %B3(13)..move along.
1160 I don't have anything to do with your little gang..move along.
1161 It's not enough that you %B3(12) have ruined your own land. Now get lost!
1162 I have something here that %B3(12) use..let me see...it's the EXIT, now get LOST!
1163 Don't you %B3(12) have your own merchants?  Whatever, I'm not selling anything to you!
1164 Members of your little "club" have ruined things around here..get lost!
1165 I don't have anything to do with your damned club..move along.
1166 Creatures like you make me sick..the things you do..get out of here Pagan!
1167 After all the things you've done..the things you believe in..leave my shop!
1168 Actions speak louder than beliefs, and I despise both your actions and all you believe in.
1169 Get out of here now!
1170 I am tolerant by nature..but infidels like you push me past my limit..get out!
1171 I cannot abide you or your actions against all that is right..BE GONE!

Look around the code for how this is used. It'll be MUCH easier to pull a deity/race/class name that way.

Also, you used rand() instead of our random system. I'd change that too. (MakeRandomInt(0, 4) for example)

Not that it matters if you're going to use the eqstr_us.txt version here, but, If you're going to use a string, may as well use it all the way and build the string as you go along as opposed to using these temporary buffers. I would c_str() it when it comes time to use it in a function that uses it instead of what you have set up right now. I know the code doesn't have this practice but it was made at a time string had performance issues. But, moreso for code cleanliness and readability, and a slight performance boost from a compiler standpoint as there's less messing with registers.

Regardless I'll see if I can get this cleaned up and put in the main branch soon. Thanks a ton!

[noudess]

I'm still learning about the code base, so I tried to change as little as I could

When you say to use the eqstr_us.txt versions, is there a mechanism for that or are you just saying to go find those strings and put them into the code like the ones that were there (I didn't pick then strings, I used what was in the code already).

Do you want me to followup with guidance and resubmit, or am I stealing what you clean up?

Thank you.

[demonstar55]

Usually we set up a define in zone/stringid.h since defines are better than just the raw number. Pretty much everything the live server sends to the client used these because its a lot better packet size wise. The Client::Message_StringID functions are what you will want to use with them, you can find examples in the code.

[Secrets]

Originally Posted by noudess

I'm still learning about the code base, so I tried to change as little as I could

When you say to use the eqstr_us.txt versions, is there a mechanism for that or are you just saying to go find those strings and put them into the code like the ones that were there (I didn't pick then strings, I used what was in the code already).

Do you want me to followup with guidance and resubmit, or am I stealing what you clean up?

Thank you.

If you could follow up yourself that'd probably do me a favor and you'd learn something in the process.

Take a look at zone/string_ids.h and the function Message_StringID. Each parameter is passed in the formatted message packet.

So when you have something like,

1159 I don't have anything to do with %B3(13)..move along.

It references a file in the client, eqstr_us.txt - 1159 is the ID in the header, and the parameters that are in the function are directly taken from the packet.

So, instead of constructing a string, you can simply do something like this:

Message_StringID(MT_WornOff, SPELL_WORN_OFF_OF,
				spells[buffs[slot].spellid].name, GetCleanName());

which is an example of a spell <name> wearing off of a mob object with <name>

SPELL_WORN_OFF_OF is ID 436 in the eqstr_us.txt file, and looks like this for the data: Your %1 spell has worn off of %2.

%1 is a variable length null terminated parameter, as is %2, and these are filled in by the server.

Hopefully that explains a bit more in detail of what exactly to do.

Also, there's tons of areas in the code like this that are missing. General rule of thumb is check if it exists in eqstr_us.txt in some form, if not, do it manually like you were doing.

Note you cannot add entries to this file - we're using the stock version of it that is included with the box set or steam download. So it must exist there, and if it's missing from one client, you should probably go ahead and do it manually.

The reason behind this file was so SOE could conserve bytes on the wire in 1999. Still, it's good practice even in 2014 to do something like this and that's why I recommend it.

[noudess]

I'll look at that tomorrow and get back to you. I'll definitely do the work. I want to learn.

I'm working on some faction rework today.

I'll post it. Not sure if you'll want it or not. The changes are based in something i found earlier and wanted to fix.

[noudess]

Working on this now. Do you have any idea which strings were used by vendors? I shouldn't have any problems with the changes, just not sure how many of them I should use. Some of the values 1172 and up look similar, but most smack more of combat than selling.

[Akkadius]

Originally Posted by noudess

Working on this now. Do you have any idea which strings were used by vendors? I shouldn't have any problems with the changes, just not sure how many of them I should use. Some of the values 1172 and up look similar, but most smack more of combat than selling.

It is very obvious that you have an ability, eye and talent for issues of this nature.

When you finalize using proper strings please create a proper pull request to our Github repo and we can analyze the changes and review for submission.

Thank you.

[Secrets]

Originally Posted by noudess

Working on this now. Do you have any idea which strings were used by vendors? I shouldn't have any problems with the changes, just not sure how many of them I should use. Some of the values 1172 and up look similar, but most smack more of combat than selling.

I believe all of the ones I listed are used in merchants. The rest are used for combat.

The ones that appear similar, but have the parameter removed, in example:

1159 I don't have anything to do with %B3(13)..move along.
1160 I don't have anything to do with your little gang..move along.

are used when a non-standard player race is used. For example, if you are browsing a merchant that dislikes you in wolf form, 1160 is used. If you are human, 1159 is used with the added parameter.

The base race is never used for these messages.

If there is only one version of the line, it is used by itself.

The only exception is "Get out of here now!" - this one is sent following "Actions speak louder than beliefs, and I despise both your actions and all you believe in." in the same transaction.
Also i've never seen the one that says 'damned' on live. Probably had a player complain about the swearing in 1997 :P

Weird, I know, but I believe that's correct.

[noudess]

Ok, this is what I learned after coding it up as you said.

1) The changes work just fine for when my char is rejected for deeds or non-std race.

2) The other messages (those that take the B3(13) are CLASS or B3(12) are RACE hard codes on client already. Thus the parameter (not sure exactly what to send) has to be a race/class indicator of some sort. Right now, sending race or class codes, or race of class plural strings yields NOCLASSERS and NORACE.

The reason I decided to fix this to begin with was that on my server there is a bard that was clearly being rejected on the server due to his deity (Bristlebane) but the code ended up making a half elf vendor say he wanted nothing to do with half elves.

I don't see any messages that support the use of the deity's name.

This leaves me with a couple of options.

• Use some of the non-argument messages for deity rejection.
• Go back to not using the standard messages and have them say what I want.

I'm giving it some thought. I like that I learned the correct way, and I've learned that those messages on the cleint side are hard coded to race/class messages.

[noudess]

The only other messages with this "Bx(n)" argument format in the header file is:

#define AA_REUSE_MSG 413 //You can use the ability %B1(1) again in %2 hour(s) %3 minute(s) %4 seconds.
#define AA_REUSE_MSG2 414 //You can use the ability %B1(1) again in %2 minute(s) %3 seconds.

And I can't find those used anywhere on the server.

I tired uppercase, lowercase, 1st letter upper on class and race and still can't get the GUI to like it. Always NOCLASSERS and UNKNOWN RACE as the arguments displayed.

Also tried using race and class codes - also no go.

[demonstar55]

I think these messages should be passed through the Say_StringID functions, which use another StringID to embed another. (so passing one of these, the first arg because the 3rd, hence the 3)

Arg 1: mob name which is passed to GENERIC_STRINGID_SAY
Arg 2: embedded StringID
Arg 3: first arg to embedded StringID
etc

[noudess]

Originally Posted by demonstar55

I think these messages should be passed through the Say_StringID functions, which use another StringID to embed another. (so passing one of these, the first arg because the 3rd, hence the 3)

Arg 1: mob name which is passed to GENERIC_STRINGID_SAY
Arg 2: embedded StringID
Arg 3: first arg to embedded StringID
etc

Changed it to this. Absolutely more correct.

I still can't find an argument I can send for %B3 that works.

With the stringIDs that have %B3 in them, none of these attempts work:

merchant->Say_StringID(messageid, merchant->GetCleanName(), "1"); * tried 1 - 100 in hopes that race/class id might work
merchant->Say_StringID(messageid, merchant->GetCleanName(), "ranger"); * tried 4 classes in lower, upper, 1st letter cap, and plural
merchant->Say_StringID(messageid, merchant->GetCleanName(), "erudite"); * tried 4 races in lower, upper, 1st letter cap and plural

They all result in:

The correct message, said by the merchant with the correct merchant name. If it was a %B3(13) is outputs NOCLASSERS and if it is a %B3(12) it outputs UNKNOWN RACE.

Obviously my last argument is wrong, and needs to supply race/class somehow.. Just don't know how.

[demonstar55]

I'll see if I can figure it out either through IDA or packet capture

Edit: also, Say_StringID handles the merchants name, should of clarified that, so you can remove the GetCleanName() (I was just trying to explain why it was a 3)

Doing Say_StringID(messageid, "12"); says Gnomes for me.

[noudess]

Originally Posted by demonstar55

I'll see if I can figure it out either through IDA or packet capture

Edit: also, Say_StringID handles the merchants name, should of clarified that, so you can remove the GetCleanName() (I was just trying to explain why it was a 3)

Doing Say_StringID(messageid, "12"); says Gnomes for me.

LOL. I saw that you're example differed from my code , but snagged an example from the code that passed GetCleanName() assuming it was correct, but that was for a different reason. I should have read your message more carefully.

Retesting and sorry,