No limit to shoulder and pelvis implants using rules assistant

Expected Behavior

limit to {type}Implants should be in the range of -2:2 depending on surgeryUpgrade falg

Current Behavior

function pelvisShouldersImplants(label, target) will expand {type}Implants infinitely

rulesAutosurgery.js:

		function pelvisShouldersImplants(label, target){
			const request = thisSurgery[`${target}Implant`];
			if (Math.abs(request) > 0) {
				if (Math.abs(request) < 2 || V.surgeryUpgrade) {
					const change = request > 0 ? 1 : -1;
					commitProcedure(`surgery to ${request > 0 ? "broaden" : "narrow"} ${his} ${label}`,
						s => { s[target] += change; s[`${target}Implant`] += change; },
						40);
				}
			}
		}

the issue is that thisSurgery[${target}Implant] is always 1 or 2 if broadening rule is set, slave[${target}Implant] is never taken into account when deciding whether to broaden/shrink or not

it might be rulesAssistantOptions.js:class HipsAndShoulderSurgeryList extends ListSelector thats at fault but i don't really know

here is what i did to fix it for myself

		function pelvisShouldersImplants(label, target){
			const request = thisSurgery[`${target}Implant`];
			if (Math.abs(request) > 0) {
				if (Math.abs(request) < 2 || V.surgeryUpgrade) {
					const change = compareValues(request, slave[`${target}Implant`]);
					if (change !== 0)
					{
						commitProcedure(`surgery to ${request > 0 ? "broaden" : "narrow"} ${his} ${label}`,
							s => { s[target] += change; s[`${target}Implant`] += change; },
							40);
					}
				}
			}
		}
		
		function compareValues(target, current) {
			if (target === current) {
				return 0;
			} else if (target > current && target > 0) {
				return 1;
			} else if (target < current && target < 0) {
				return -1;
			}
		}

Additional information

mod version: 4.0.0-alpha.28, build: 1219, commit: dcad3920

You don't need elseif if you return after each condition. More over, if neither of the three condition matches, the function will return nothing. It should instead raise an error or warning, that way if the API changes, the function will self report the issue.

all true, it was only meant as a showcase of how the bug can be quickfixed, it wasn't really a suggestion of a proper fix

here is a nicer version inspired by function bodyPartSizing

function pelvisShouldersImplants(label, target){
	const current = slave[`${target}Implant`]
	const request = thisSurgery[`${target}Implant`];
	const shouldNarrow = current > request; // && current <= 0    //adding those 2 conditions would make it consistent with manual broadening/narrowing 
	const shouldBroaden = current < request; // && current >= 0   //cuz you cant manually narrow a broadend pelvis and vice versa
	const canPerformSurgery = Math.abs(request) > 0 && (Math.abs(request) < 2 || V.surgeryUpgrade);
	
	if ((!shouldNarrow && !shouldBroaden) || !canPerformSurgery) {
		return;
	}
	
	const change = shouldBroaden ? 1 : shouldNarrow ? -1 : 0;
	if (change === 0) {
		return;
	}
	
	commitProcedure(`surgery to ${request > 0 ? "broaden" : "narrow"} ${his} ${label}`,
		s => { s[target] += change; s[`${target}Implant`] += change; },
		40);
}

You're one step closer to a Merge Request :P

closed