Max enchant behaviour improvement

This is not a Support area! Discuss about the Server here. Non-Server related discussion goes in Off-Topic Discussion.
Forum rules
READ NOW: L2j Forums Rules of Conduct
Post Reply
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Max enchant behaviour improvement

Post by Ghurdyl »

Hello,

Was studying the maximum enchant handling this morning and I found what I consider as an incoherence.

The max enchant value is set in two places, first in the configuration and also in the AbstractEnchantPacket class.(com.l2jserver.gameserver.network.clientpackets)

I think the behavior goes wrong beacause in AbstractEnchantPacket we have a per scroll max enchant control while in the configuration you set a value for all the scrolls at once.
Right now the configuration value overrules the values in AbstractEnchantPacket unless the AbstractEnchantPacket is lower than config value.

I think it would be better to use configuration as a default configuration and if the max enchant set in AbstractEnchantPacket is different from 0 then it overrules the config.

I noticed this because I was implementing the Master of Enchantement Event.
In this event, it has to be possible to reach a +23 enchant. But our server has limited the max to +18.
So to do this, we had to increase the default enchant up to +23 and narrow this value (in the AbstractEnchantPacket) to +18 for all single scroll enchant except the special scroll for this event. (13540)

I will fix this myself for our server but I wanted to share my point of view and maybe the patch if there are interested people.

Cheers,

Ghurdyl :D
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
User avatar
jurchiks
Posts: 6769
Joined: Sat Sep 19, 2009 4:16 pm
Location: Eastern Europe

Re: Max enchant behaviour improvement

Post by jurchiks »

This has been discussed in this forum, I even suggested adding a "custom enchant rate system per item", but it never got any replies.
Apply this:

Code: Select all

Index: java/com/l2jserver/gameserver/network/clientpackets/AbstractEnchantPacket.java===================================================================--- java/com/l2jserver/gameserver/network/clientpackets/AbstractEnchantPacket.java	(revision 4075)+++ java/com/l2jserver/gameserver/network/clientpackets/AbstractEnchantPacket.java	(working copy)@@ -63,7 +63,7 @@ 				// weapon scrolls can enchant only weapons 				case L2Item.TYPE2_WEAPON: 					if (!_isWeapon-							|| (Config.ENCHANT_MAX_WEAPON > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_WEAPON))+							|| (Config.ENCHANT_MAX_WEAPON > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_WEAPON && enchantItem.getItemId() != 13539)) 						return false; 					break; 					// armor scrolls can enchant only accessory and armors
and max enchant won't affect yogi's scroll.

EnchantChanceWeapon = 66
EnchantChanceArmor = 66
EnchantChanceJewelry = 66
EnchantChanceElement = 50
BlessedEnchantChanceWeapon = 66
BlessedEnchantChanceArmor = 66
BlessedEnchantChanceJewelry = 66
EnchantMaxWeapon = 16
EnchantMaxArmor = 8
EnchantMaxJewelry = 8
where do you see "a value for all the scrolls at once"?
If you have problems, FIRST TRY SOLVING THEM YOURSELF, and if you get errors, TRY TO ANALYZE THEM, and ONLY if you can't help it, THEN ask here.
Otherwise you will never learn anything if all you do is copy-paste!
Discussion breeds innovation.
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Re: Max enchant behaviour improvement

Post by Ghurdyl »

Thanks for the answer.
jurchiks wrote:where do you see "a value for all the scrolls at once"?
in the configuration (character.properties)

Code: Select all

@310# This is the enchant limit, if set to 0, there will be no limit.# Example: If this is set to 10, the maximum enchant will be 10.# Default: 0, 0, 0EnchantMaxWeapon = 23EnchantMaxArmor = 18EnchantMaxJewelry = 18
in AbstractEnchantPacket you have a fine grained configuration :

Code: Select all

@ 200		static	{		// itemId, (isWeapon, isBlessed, isCrystal, isSafe, grade, max enchant level, chance increase, allowed item IDs)		// allowed items list must be sorted by ascending order		_scrolls.put(729, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(730, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(731, new EnchantScroll(true, false, true, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(732, new EnchantScroll(false, false, true, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(947, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(948, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(949, new EnchantScroll(true, false, true, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(950, new EnchantScroll(false, false, true, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(951, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(952, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(953, new EnchantScroll(true, false, true, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(954, new EnchantScroll(false, false, true, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(955, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(956, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(957, new EnchantScroll(true, false, true, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(958, new EnchantScroll(false, false, true, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(959, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(960, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(961, new EnchantScroll(true, false, true, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(962, new EnchantScroll(false, false, true, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(6569, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(6570, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_A, 18, 0, null));		_scrolls.put(6571, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(6572, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_B, 18, 0, null));		_scrolls.put(6573, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(6574, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_C, 18, 0, null));		_scrolls.put(6575, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(6576, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_D, 18, 0, null));		_scrolls.put(6577, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(6578, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_S, 18, 0, null));		_scrolls.put(22006, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_D, 18, 10, null));		_scrolls.put(22007, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_C, 18, 10, null));		_scrolls.put(22008, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_B, 18, 10, null));		_scrolls.put(22009, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_A, 18, 10, null));		_scrolls.put(22010, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_D, 18, 10, null));		_scrolls.put(22011, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_C, 18, 10, null));		_scrolls.put(22012, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_B, 18, 10, null));		_scrolls.put(22013, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_A, 18, 10, null));		_scrolls.put(22014, new EnchantScroll(true, false, false, true, L2Item.CRYSTAL_B, 16, 10, null));		_scrolls.put(22015, new EnchantScroll(true, false, false, true, L2Item.CRYSTAL_A, 16, 10, null));		_scrolls.put(22016, new EnchantScroll(false, false, false, true, L2Item.CRYSTAL_B, 16, 10, null));		_scrolls.put(22017, new EnchantScroll(false, false, false, true, L2Item.CRYSTAL_A, 16, 10, null));		_scrolls.put(22018, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_B, 18, 100, null));		_scrolls.put(22019, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_A, 18, 100, null));		_scrolls.put(22020, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_B, 18, 100, null));		_scrolls.put(22021, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_A, 18, 100, null)); 		// Master Yogi's Scroll Enchant Weapon (event)		_scrolls.put(13540, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_NONE, 0, 0, new int[]{ 13539 }));
This is our modified code for handling default max +18 and special max +23 (config set max weapon to +23)

It would be better to keep the "0" value where I set an "18" and only change the master yogi' scroll to 23.
But in the current behavior the +23 wouldn't be taken in account as the config value would override it with +18.
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
User avatar
jurchiks
Posts: 6769
Joined: Sat Sep 19, 2009 4:16 pm
Location: Eastern Europe

Re: Max enchant behaviour improvement

Post by jurchiks »

well, it IS weird, but the fact is that afaik this doesn't work/affect anything, since the enchant limit is like set in configs
If you have problems, FIRST TRY SOLVING THEM YOURSELF, and if you get errors, TRY TO ANALYZE THEM, and ONLY if you can't help it, THEN ask here.
Otherwise you will never learn anything if all you do is copy-paste!
Discussion breeds innovation.
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Re: Max enchant behaviour improvement

Post by Ghurdyl »

And that is precisely what I am going to change.

Right now the config overrules any higher value than the one set in the config.
If in the "_scrolls" list an EnchantScroll defines a lower value than in the config, this value will overrule config.
That is because the method EnchantItem.isValid(L2ItemInstance enchantItem) was wrong.

Here is the current method :

Code: Select all

public final boolean isValid(L2ItemInstance enchantItem){	if (enchantItem == null)		return false; 	int type2 = enchantItem.getItem().getType2(); 	// checking scroll type and configured maximum enchant level	switch (type2)	{		// weapon scrolls can enchant only weapons		case L2Item.TYPE2_WEAPON:			if (!_isWeapon					|| (Config.ENCHANT_MAX_WEAPON > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_WEAPON))				return false;			break;			// armor scrolls can enchant only accessory and armors		case L2Item.TYPE2_SHIELD_ARMOR:			if (_isWeapon					|| (Config.ENCHANT_MAX_ARMOR > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_ARMOR))				return false;			break;		case L2Item.TYPE2_ACCESSORY:			if (_isWeapon					|| (Config.ENCHANT_MAX_JEWELRY > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_JEWELRY))				return false;			break;		default:			return false;	} 	// check for crystal types	if (_grade != enchantItem.getItem().getItemGradeSPlus())		return false; 	// check for maximum enchant level	if (_maxEnchantLevel != 0 && enchantItem.getEnchantLevel() >= _maxEnchantLevel)		return false; 	if(_itemIds != null && Arrays.binarySearch(_itemIds, enchantItem.getItemId()) < 0)		return false; 	return true;}
As you can see, the _maxEnchantLevel is taken in account only if it passes the test against the config value.

And here is what came out of my fast frist shot (not tested yet)

Code: Select all

public final boolean isValid(L2ItemInstance enchantItem){	if (enchantItem == null)		return false; 	int type2 = enchantItem.getItem().getType2(); 	// check for max level	int maxLvl = getMaxEnchantLevel(type2);	if(maxLvl > 0 && enchantItem.getEnchantLevel() >= maxLvl) {		return false;	} 	// checking scroll type	switch (type2)	{		// weapon scrolls can enchant only weapons		case L2Item.TYPE2_WEAPON:			if (!_isWeapon)				return false;			break;			// armor scrolls can enchant only accessory and armors		case L2Item.TYPE2_SHIELD_ARMOR:		case L2Item.TYPE2_ACCESSORY:			if (_isWeapon)				return false;			break;		default:			return false;	} 	// check for crystal types	if (_grade != enchantItem.getItem().getItemGradeSPlus())		return false; 	// check for item id restriction	if(_itemIds != null && Arrays.binarySearch(_itemIds, enchantItem.getItemId()) < 0)		return false; 	return true;} public final int getMaxEnchantLevel(int itemType){	if(_maxEnchantLevel != 0) {		return _maxEnchantLevel;	}	else {			// checking scroll type and configured maximum enchant level		switch (itemType)		{			case L2Item.TYPE2_WEAPON:				return Config.ENCHANT_MAX_WEAPON;			case L2Item.TYPE2_SHIELD_ARMOR:				return Config.ENCHANT_MAX_ARMOR;			case L2Item.TYPE2_ACCESSORY:				return Config.ENCHANT_MAX_JEWELRY;			default:				return 1;		}	}}
I added a getMaxEnchantLevel(int itemType) which returns the _maxEnchantLevel if set or the config value if not set.

I am going to test this out and see how it its going.
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
User avatar
jurchiks
Posts: 6769
Joined: Sat Sep 19, 2009 4:16 pm
Location: Eastern Europe

Re: Max enchant behaviour improvement

Post by jurchiks »

we'll see what you make out of it :)
i'm busy with other things atm, rewriting quests and events in java...
If you have problems, FIRST TRY SOLVING THEM YOURSELF, and if you get errors, TRY TO ANALYZE THEM, and ONLY if you can't help it, THEN ask here.
Otherwise you will never learn anything if all you do is copy-paste!
Discussion breeds innovation.
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Re: Max enchant behaviour improvement

Post by Ghurdyl »

As far as I tested it, this woks just fine :D

So here is the patch

Code: Select all

Index: AbstractEnchantPacket.java===================================================================--- AbstractEnchantPacket.java	(revision 4062)+++ AbstractEnchantPacket.java	(working copy)@@ -56,25 +56,25 @@ 				return false;  			int type2 = enchantItem.getItem().getType2();--			// checking scroll type and configured maximum enchant level+			+			// check for max level+			int maxLvl = getMaxEnchantLevel(type2);+			if(maxLvl > 0 && enchantItem.getEnchantLevel() >= maxLvl) {+				return false;+			}+			+			// checking scroll type 			switch (type2) 			{ 				// weapon scrolls can enchant only weapons 				case L2Item.TYPE2_WEAPON:-					if (!_isWeapon-							|| (Config.ENCHANT_MAX_WEAPON > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_WEAPON))+					if (!_isWeapon) 						return false; 					break; 					// armor scrolls can enchant only accessory and armors 				case L2Item.TYPE2_SHIELD_ARMOR:-					if (_isWeapon-							|| (Config.ENCHANT_MAX_ARMOR > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_ARMOR))-						return false;-					break; 				case L2Item.TYPE2_ACCESSORY:-					if (_isWeapon-							|| (Config.ENCHANT_MAX_JEWELRY > 0 && enchantItem.getEnchantLevel() >= Config.ENCHANT_MAX_JEWELRY))+					if (_isWeapon) 						return false; 					break; 				default:@@ -84,11 +84,8 @@ 			// check for crystal types 			if (_grade != enchantItem.getItem().getItemGradeSPlus()) 				return false;--			// check for maximum enchant level-			if (_maxEnchantLevel != 0 && enchantItem.getEnchantLevel() >= _maxEnchantLevel)-				return false;-+			+			// check for item id restriction 			if(_itemIds != null && Arrays.binarySearch(_itemIds, enchantItem.getItemId()) < 0) 				return false; @@ -102,6 +99,30 @@ 		{ 			return _chanceAdd; 		}+		+		/*+		 * returns the max enchant+		 */+		public final int getMaxEnchantLevel(int itemType)+		{+			if(_maxEnchantLevel != 0) {+				return _maxEnchantLevel;+			}+			else {	+				// checking scroll type and configured maximum enchant level+				switch (itemType)+				{+					case L2Item.TYPE2_WEAPON:+						return Config.ENCHANT_MAX_WEAPON;+					case L2Item.TYPE2_SHIELD_ARMOR:+						return Config.ENCHANT_MAX_ARMOR;+					case L2Item.TYPE2_ACCESSORY:+						return Config.ENCHANT_MAX_JEWELRY;+					default:+						return 1;+				}+			}+		} 	}  	public static final class EnchantScroll extends EnchantItem@@ -231,6 +252,8 @@ 		_scrolls.put(6576, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_D, 0, 0, null)); 		_scrolls.put(6577, new EnchantScroll(true, true, false, false, L2Item.CRYSTAL_S, 0, 0, null)); 		_scrolls.put(6578, new EnchantScroll(false, true, false, false, L2Item.CRYSTAL_S, 0, 0, null));+		// Master Yogi's Scroll Enchant Weapon (event)+		_scrolls.put(13540, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_NONE, 0, 0, new int[]{ 13539 })); 		_scrolls.put(22006, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_D, 0, 10, null)); 		_scrolls.put(22007, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_C, 0, 10, null)); 		_scrolls.put(22008, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_B, 0, 10, null));@@ -248,9 +271,6 @@ 		_scrolls.put(22020, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_B, 0, 100, null)); 		_scrolls.put(22021, new EnchantScroll(false, false, false, false, L2Item.CRYSTAL_A, 0, 100, null)); -		// Master Yogi's Scroll Enchant Weapon (event)-		_scrolls.put(13540, new EnchantScroll(true, false, false, false, L2Item.CRYSTAL_NONE, 0, 0, new int[]{ 13539 }));- 		// itemId, (isWeapon, grade, max enchant level, chance increase) 		_supports.put(12362, new EnchantItem(true, L2Item.CRYSTAL_D, 9, 20, null)); 		_supports.put(12363, new EnchantItem(true, L2Item.CRYSTAL_C, 9, 18, null)); 
As reminder :
This patch changes the behavior of the max enchant allowed :
When a scroll defines a max enchant level different from 0, it will be used otherwise, the value defined by the configuration will be used.
So
if max enchant = 0 and configuration for the item type = 0 => then no enchant limit.
if max enchant = 0 and configuration for the item type = x => then enchant limit = x
if max enchant = x(!=0) => then enchant limit = x

(I allowed myself to "reorder" the "_scrolls.put" order as a comment above said that scrolls have to be added in an ascending way. Master yogi' scroll broke this rule.)

One las better improvement would be to populate this "_scrolls" list from the configuration so that no Java code would be needed to change a max enchant value, a chance bonus or an item restriction.
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
User avatar
jurchiks
Posts: 6769
Joined: Sat Sep 19, 2009 4:16 pm
Location: Eastern Europe

Re: Max enchant behaviour improvement

Post by jurchiks »

well the master yogi scroll relocation was really just cosmetic...
but imo that "per scroll" enchant limit should be just removed at all, imho nobody uses it and it would be truly dumb to set a different enchant level for every scroll, so...
If you have problems, FIRST TRY SOLVING THEM YOURSELF, and if you get errors, TRY TO ANALYZE THEM, and ONLY if you can't help it, THEN ask here.
Otherwise you will never learn anything if all you do is copy-paste!
Discussion breeds innovation.
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Re: Max enchant behaviour improvement

Post by Ghurdyl »

I am a perfectionist :P so even cosmetic changes are important to me :)

The possibility was there to allow a per-scroll customization, so why should I not make it fully working ?

BTW this functionality IS used and not by only me, see those 4 lines :
_scrolls.put(22014, new EnchantScroll(true, false, false, true, L2Item.CRYSTAL_B, 16, 10, null));
_scrolls.put(22015, new EnchantScroll(true, false, false, true, L2Item.CRYSTAL_A, 16, 10, null));
_scrolls.put(22016, new EnchantScroll(false, false, false, true, L2Item.CRYSTAL_B, 16, 10, null));
_scrolls.put(22017, new EnchantScroll(false, false, false, true, L2Item.CRYSTAL_A, 16, 10, null));
There is a limit to +16 and it is like this on the SVN, it's none of mine. :wink:
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
User avatar
jurchiks
Posts: 6769
Joined: Sat Sep 19, 2009 4:16 pm
Location: Eastern Europe

Re: Max enchant behaviour improvement

Post by jurchiks »

I am too, but the Yogi's scroll is a special case as it enchants only one item and not a group of items...
It's because of Random that there is no point in "per scroll" rate, how many times have you heard players saying "this item enchants better than that item" even though the rates are the same?
I would understand if it was a minimum value for enchant logging though, then it would make sense, otherwise "no thanks"...
If you have problems, FIRST TRY SOLVING THEM YOURSELF, and if you get errors, TRY TO ANALYZE THEM, and ONLY if you can't help it, THEN ask here.
Otherwise you will never learn anything if all you do is copy-paste!
Discussion breeds innovation.
Ghurdyl
Posts: 55
Joined: Tue Jun 03, 2008 4:52 pm
Location: Hannut, Belgium

Re: Max enchant behaviour improvement

Post by Ghurdyl »

I think this could be needed for come custom server configuration.

Maybe someone could wish to limit blessed sroll enchant to +15 while normal scrolls will go higher or the opposite.
they could also invent a per-grade enchant rule as high grade weapons are harder to find they could be easier to enchant up to a certain level with certain scrolls.
There are plenty of customs scroll and ideas people could have around this.
We should not underestimate peoples' imagination.

This is definitely a very very very small enhancement but why on earth having anything against it ?!

Anyway I am not forcing anyone to use it. It is there, it works fine and it's free to use.

Done.
~Ghurdyl~
"If you give a man a fire, he'll be warm for a day. If you set a man on fire, he'll be warm for the rest of his life" :)
Post Reply