Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not really compatible with xskills -- Blasksmith skill completely being overwritten. #1

Open
Diometus opened this issue Sep 16, 2023 · 4 comments

Comments

@Diometus
Copy link

Diometus commented Sep 16, 2023

Whenever a person learns via xskills the Blacksmith skill, the rate of getting items that are above common drastically diminishes; however, whenever one does not have the skill, they get at what feels like a very significantly higher chance of getting items to roll greater than common. It's almost like it is not really compatible with xskills' Blacksmith skill. On top of this, when blasksmithing using the Blacksmith skill, the if the blacksmith crafted an item that has a higher quality with better durability, and then drops or uses the item, it is overwritten with RPG Item Rarity and rolls significantly lower stats and gravely lesser quality than what the blacksmith has crafted.
Also posted this here: your forum post and here: your mod page

@YewYew
Copy link
Owner

YewYew commented Sep 17, 2023

Thanks for the issue post.

This is something I am aware of, and know of a way to fix. However, I currently lack the time to implement. I do plan on getting to it eventually.

When I added the existing "compatibility", I'll admit my knowledge about XSkills was lacking because I don't use it.

Currently, the compatibility is just where rarity is influenced by the quality. Thus, items XSkills doesn't give the 'quality' attribute are subjected to normal RPGItemRarity, and since RPGItemRarity currently takes precedent in stat modification it overrides XSkills using 'quality' as the 'rarity'.

The "fix" I have in mind (in the case anyone is curious or would like to do a pr themselves) would be the following:

Instead of having the 'quality' attribute be the 'rarity' attribute when it exists, it would generate 'rarity' normally. If it is lower then the 'quality', it would use the 'quality' instead. Then, ideally it would assign stats based on XSkills modifiers instead of those programmed in RPGItemRarity. It would use RPGItemRarity modifiers for anything XSkills doesn't change. This should solve any issues.

One issue with compatibility is XSkills source doesn't appear to be anywhere, so it is a smidgen more difficult then it should be, mostly about stuff concerning stat modifiers.

@Diometus
Copy link
Author

Thanks for the issue post.

This is something I am aware of, and know of a way to fix. However, I currently lack the time to implement. I do plan on getting to it eventually.

When I added the existing "compatibility", I'll admit my knowledge about XSkills was lacking because I don't use it.

Currently, the compatibility is just where rarity is influenced by the quality. Thus, items XSkills doesn't give the 'quality' attribute are subjected to normal RPGItemRarity, and since RPGItemRarity currently takes precedent in stat modification it overrides XSkills using 'quality' as the 'rarity'.

The "fix" I have in mind (in the case anyone is curious or would like to do a pr themselves) would be the following:

Instead of having the 'quality' attribute be the 'rarity' attribute when it exists, it would generate 'rarity' normally. If it is lower then the 'quality', it would use the 'quality' instead. Then, ideally it would assign stats based on XSkills modifiers instead of those programmed in RPGItemRarity. It would use RPGItemRarity modifiers for anything XSkills doesn't change. This should solve any issues.

One issue with compatibility is XSkills source doesn't appear to be anywhere, so it is a smidgen more difficult then it should be, mostly about stuff concerning stat modifiers.

Well, if you can't seem to get in contact with xSkills mod author about that, there is always ILSpy that can be used to open the dll and see if the information is in the dll, right? It's not like your modifying or trying to change anything but understand their mod so you can make compatibility with it in your own so it shouldn't be a breech of anything since its an educational method of teaching oneself to understand a program better from a Computer Science perspective.

@YewYew
Copy link
Owner

YewYew commented Sep 17, 2023

Well, if you can't seem to get in contact with xSkills mod author about that, there is always ILSpy that can be used to open the dll and see if the information is in the dll, right? It's not like your modifying or trying to change anything but understand their mod so you can make compatibility with it in your own so it shouldn't be a breech of anything since its an educational method of teaching oneself to understand a program better from a Computer Science perspective.

I had the same thoughts. I'm going try contacting the mod author once I know exactly what information I need, then try dnspy/Ilspy if necessary. I have used it on VS mods before, and it works fine.

@Vlammar
Copy link

Vlammar commented Oct 6, 2023

I have take a look at xskill and xlib, what I have found is that the part where the blacksmith level is used to increase the
It's in BlockEntityAnvilPatch

	{
		//IL_01c2: Unknown result type (might be due to invalid IL or missing references)
		//IL_01cc: Expected O, but got Unknown
		__state = new AnvilState(__instance);
		if (__state.recipe == null)
		{
			return true;
		}
		PlayerSkill playerSkill = null;
		float num = -1f;
		if (__state.metalworking == null || __state.workItemStack == null)
		{
			return true;
		}
		if (byPlayer == null)
		{
			byPlayer = __instance.GetUsedByPlayer();
			object obj;
			if (byPlayer == null)
			{
				obj = null;
			}
			else
			{
				EntityPlayer entity = byPlayer.Entity;
				obj = ((entity == null) ? null : ((Entity)entity).GetBehavior<PlayerSkillSet>()?[__state.metalworking.Id]);
			}
			playerSkill = (PlayerSkill)obj;
			if (playerSkill == null)
			{
				return true;
			}
			PlayerAbility playerAbility = playerSkill[__state.metalworking.MachineLearningId];
			if (playerAbility != null && playerAbility.Tier <= 0)
			{
				return true;
			}
		}
		playerSkill = playerSkill ?? ((Entity)byPlayer.Entity).GetBehavior<PlayerSkillSet>()?[__state.metalworking.Id];
		PlayerAbility playerAbility2 = playerSkill?[__state.metalworking.FinishingTouchId];
		if (playerAbility2 == null)
		{
			return true;
		}
		if (playerAbility2.Tier > 0)
		{
			num = __instance.FinishedProportion();
			if ((double)(Math.Min((float)playerAbility2.Value(0) + (float)playerAbility2.Value(1) * 0.1f, playerAbility2.Value(2)) * 0.01f * num * num) >= ((Entity)byPlayer.Entity).World.Rand.NextDouble())
			{
				__state.splitCount += __instance.FinishRecipe();
				num = 1f;
			}
		}
		playerAbility2 = playerSkill[__state.metalworking.HeatingHitsId];
		if (playerAbility2 == null || __state.anvilItemStack == null)
		{
			return true;
		}
		float temperature = __state.workItemStack.Collectible.GetTemperature(((BlockEntity)__instance).Api.World, __state.workItemStack);
		float meltingPoint = __state.workItemStack.Collectible.GetMeltingPoint(((BlockEntity)__instance).Api.World, (ISlotProvider)null, (ItemSlot)new DummySlot(__state.anvilItemStack.GetBaseMaterial(__state.workItemStack)));
		temperature = ((!(meltingPoint > 0f)) ? (temperature + (float)playerAbility2.Value(0)) : Math.Min(temperature + (float)playerAbility2.Value(0), meltingPoint));
		__state.workItemStack.Collectible.SetTemperature(((BlockEntity)__instance).Api.World, __state.workItemStack, temperature, true);
		playerAbility2 = playerSkill[__state.metalworking.BlacksmithId];
		if (playerAbility2.Tier > 0 && (!__state.wasPlate || !(((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Collectible is ItemMetalPlate)) && !(((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Collectible is ItemIngot))
		{
			float num2 = (float)playerAbility2.Value(0) * ((float)Math.Min(playerSkill.Level, 20) * 0.25f);
			num2 *= 0.5f;
			num2 = Math.Min(num2 + (float)((Entity)byPlayer.Entity).World.Rand.NextDouble() * num2, playerAbility2.Value(1));
			((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Attributes.SetFloat("quality", num2);
		}
		if (num >= 1f || num < 0f || byPlayer == null)
		{
			return true;
		}
		return false;
	} 

This part is the interesting stuff

		if (playerAbility2.Tier > 0 && (!__state.wasPlate || !(((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Collectible is ItemMetalPlate)) && !(((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Collectible is ItemIngot))
		{
			float num2 = (float)playerAbility2.Value(0) * ((float)Math.Min(playerSkill.Level, 20) * 0.25f);
			num2 *= 0.5f;
			num2 = Math.Min(num2 + (float)((Entity)byPlayer.Entity).World.Rand.NextDouble() * num2, playerAbility2.Value(1));
			((RecipeBase<SmithingRecipe>)(object)__state.recipe).Output.ResolvedItemstack.Attributes.SetFloat("quality", num2);
		}

From what I understand if the skill is >0 then we directly give the item. The behavior without the skill is different because (see below) it tries to give the item using the vintage story API

if (num > 0f)
				{
					val3.Attributes.SetFloat("quality", num);
				}
				if (flag2 || !byPlayer.InventoryManager.TryGiveItemstack(val3, false))
				{
					((BlockEntity)__instance).Api.World.SpawnItemEntity(val3, ((BlockEntity)__instance).Pos.ToVec3d().Add(0.5, 1.5, 0.5), (Vec3d)null);
				}
			}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants