machina.js icon indicating copy to clipboard operation
machina.js copied to clipboard

I'll submit a PR if you confirm this is a bug

Open sfrooster opened this issue 3 years ago • 0 comments

I'm in the process of creating type definitions for this library (which, BTW, I still find to be the best FSM lib I've found among those available for 3 different languages - node/javascript, dotnet/C#, and golang) which I'll submit to DefinitelyTyped (as a further ambition I may submit a PR to update the docs, but don't hold your breath) and in looking at the function getHandlerArgs which is defined in both the FSM and the BehavioralFSM, I noticed what looks to be a typo or c/p error or.... in the FSM version.

For comparison, in the BehavioralFSM:

getHandlerArgs: function( args, isCatchAll ) {
	// index 0 is the client, index 1 is inputType
	// if we're in a catch-all handler, input type needs to be included in the args
	// inputType might be an object, so we need to just get the inputType string if so
	var _args = args.slice( 0 );
	var input = _args[ 1 ];					// <=== here, input is defined and set
	if ( typeof input === "object" ) {			// <=== here, input is used
		_args.splice( 1, 1, input.inputType );
	}
	return isCatchAll ?
		_args :
		[ _args[ 0 ] ].concat( _args.slice( 2 ) );
}

and in the FSM:

getHandlerArgs: function( args, isCatchAll ) {
	// index 0 is the client, index 1 is inputType
	// if we're in a catch-all handler, input type needs to be included in the args
	// inputType might be an object, so we need to just get the inputType string if so
	var _args = args;
	var input = _args[ 1 ];					// <=== here, input is (also) defined and set
	if ( typeof inputType === "object" ) {			// <=== but here, inputType is used
		_args.splice( 1, 1, input.inputType );
	}
	return isCatchAll ?
		_args.slice( 1 ) :
		_args.slice( 2 );
}

I'm currently working with a BehavioralFSM so it'd take some work to get an FSM going and test etc, but I've looked at enclosing scopes and I don't see inputType coming in as a closure or anything like that so I think it's really meant to be input vs inputType and just hasn't been noticed.

If you agree, I'm happy to submit a PR, I just didn't want to do so without confirming I'm not missing something.

sfrooster avatar Nov 28 '21 06:11 sfrooster