[Engine V2] step field on actions

[michaeljb]

#12193

This PR depends on #12217.

Before clicking "Create"

• Branch is derived from the latest master
• Add the pins or archive_alpha_games label if this change will break existing games
• Code passes linter with docker compose exec rack rubocop -a
• Tests pass cleanly with docker compose exec rack rake

Implementation Notes

Explanation of Change

• Action::Base
  • add a step attr to Action objects
• Round::Base
  • after processing an action, set the Action's step
  • before processing an action, if it has a step, use it to quickly select which Step to use; otherwise, use the old/non-lazy method of checking all Steps in order for one that can process the given action, and throwing an error if a blocking Step that cannot process the action is found
• update 1889 fixtures
  • add step to actions
  • some undo and redo actions have been removed
• if the Game's use_engine_v2 is not true, step is not set on any actions, and the way Round processes actions is unchanged

Screenshots

1889 fixture showing faster processing with the step field changes. Legacy on top (via v2=f) processed the actions in 0.379 seconds, and Engine V2 (v2=t) on the bottom took 0.263 seconds:

The above screenshot is one of many trial results:

Any Assumptions / Hacks

• the high-level assumption is that once a step has processed an action, it is safe to rely on that same step to process the action when the game is processed in the future
  • if there is a bug fix that should break a game by introducing a blocking step where there wasn't one before, that game will be processed by the engine without error, even though we want an error
    • a TODO comment has been added to address this, and a checklist item has been added to #12193.

[ollybh]

This looks like a good change to me, though I've got a suggestion for a tweaked algorithm for calculating the lookup hash for steps.

[ollybh]

This seems over-optimised for a block that's going to get called just once a round. Most of the code on both branches of the if statement is repeated.

Suggested change

	if grouped_steps.one?
	step = grouped_steps[0]
	steps_h[step_type] = step
	steps_h[step] = step_type
	else
	grouped_steps.each.with_index do |grouped_step, index|
	key =
	if index.zero?
	step_type
	else
	"#{step_type}#{index + 1}"
	end
	steps_h[key] = grouped_step
	steps_h[grouped_step] = key
	end
	grouped_steps.each_with_index do |grouped_step, index|
	key = "#{step_type}#{index.zero? ? '' : index + 1}"
	steps_h[key] = grouped_step
	steps_h[grouped_step] = key
	end

I've tested this and it's a few milliseconds faster than your code, mostly because each_with_index is more efficient that each.with_index (it doesn't need to create an Iterator). The change in the inner block is basically the same speed.