# HG changeset patch
# User Michael Granger <ged@FaerieMUD.org>
# Date 1630534848 25200
#      Wed Sep 01 15:20:48 2021 -0700
# Node ID 5a383cbf5393d7b47b8dd3a7fd63edf5a5b225ca
# Parent  bff33f97f1c2f2c4b12ceccbb07d08d0533f2a12
Rework how state data is passed around.

Instead of passing it through the transition callbacks, which proved to be a terrible idea, the
States themselves take an optional state argument to their constructor.

diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -31,18 +31,19 @@
     $ gem install pushdown
 
 
-## Contributing
+## Development
 
-You can check out the current development source with Mercurial via its
-[project page](http://bitbucket.org/ged/pushdown). Or if you prefer
-Git, via [its Github mirror](https://github.com/ged/pushdown).
+You can check out the current source with Git via Gitlab:
+
+    $ hg clone http://hg.sr.ht/~ged/Pushdown
+    $ cd Pushdown
 
 After checking out the source, run:
 
-    $ rake newb
+    $ gem install -Ng
+    $ rake setup
 
-This task will install any missing dependencies, run the tests/specs,
-and generate the API documentation.
+This task will install dependencies, and do any other necessary setup for development.
 
 
 ## Author(s)
diff --git a/lib/pushdown/state.rb b/lib/pushdown/state.rb
--- a/lib/pushdown/state.rb
+++ b/lib/pushdown/state.rb
@@ -57,31 +57,46 @@
 	end
 
 
+	### Set up new States with an optional +data+ object.
+	def initialize( data=nil )
+		@data = data
+	end
+
+
+	######
+	public
+	######
+
+	##
+	# The state data object that was used to create the State (if any)
+	attr_reader :data
+
+
 	#
 	# Stack callbacks
 	#
 
 	### Stack callback -- called when the state is added to the stack.
-	def on_start( data=nil )
+	def on_start
 		return nil # no-op
 	end
 
 
 	### Stack callback -- called when the state is removed from the stack.
-	def on_stop( data=nil )
+	def on_stop
 		return nil # no-op
 	end
 
 
 	### Stack callback -- called when another state is pushed over this one.
-	def on_pause( data=nil )
+	def on_pause
 		return nil # no-op
 	end
 
 
 	### Stack callback -- called when another state is popped off from in front of
 	### this one, making it the current state.
-	def on_resume( data=nil )
+	def on_resume
 		return nil # no-op
 	end
 
@@ -146,7 +161,10 @@
 
 		if state_class_name
 			state_class = automaton.class.pushdown_state_class( stack_name, state_class_name )
-			return Pushdown::Transition.create( transition_type, transition_name, state_class )
+			state_data = self.data
+
+			return Pushdown::Transition.
+				create( transition_type, transition_name, state_class, state_data )
 		else
 			return Pushdown::Transition.create( transition_type, transition_name )
 		end
diff --git a/lib/pushdown/transition.rb b/lib/pushdown/transition.rb
--- a/lib/pushdown/transition.rb
+++ b/lib/pushdown/transition.rb
@@ -35,12 +35,9 @@
 	end
 
 
-	### Create a new Transition with the given +name+. If +data+ is specified, it will be passed
-	### through the transition callbacks on State (State#on_start, State#on_stop, State#on_pause,
-	### State#on_resume) as it is applied.
-	def initialize( name, data=nil )
+	### Create a new Transition with the given +name+.
+	def initialize( name )
 		@name = name
-		@data = data
 	end
 
 
@@ -52,10 +49,6 @@
 	# The name of the transition; mostly for human consumption
 	attr_reader :name
 
-	##
-	# Data to pass to the transition callbacks when applying this Transition.
-	attr_accessor :data
-
 
 	### Return a state +stack+ after the transition has been applied.
 	def apply( stack )
diff --git a/lib/pushdown/transition/pop.rb b/lib/pushdown/transition/pop.rb
--- a/lib/pushdown/transition/pop.rb
+++ b/lib/pushdown/transition/pop.rb
@@ -26,8 +26,8 @@
 
 		self.log.debug "popping a state"
 		@popped_state = stack.pop
-		self.data = @popped_state.on_stop( self.data )
-		stack.last.on_resume( self.data )
+		@popped_state.on_stop
+		stack.last.on_resume
 
 		return stack
 	end
diff --git a/lib/pushdown/transition/push.rb b/lib/pushdown/transition/push.rb
--- a/lib/pushdown/transition/push.rb
+++ b/lib/pushdown/transition/push.rb
@@ -11,9 +11,11 @@
 
 	### Create a transition that will Push an instance of the given +state_class+ to
 	### the stack.
-	def initialize( name, state_class, *args )
-		super( name, *args )
+	def initialize( name, state_class, data=nil )
+		super( name )
+
 		@state_class = state_class
+		@data = data
 	end
 
 
@@ -25,15 +27,19 @@
 	# The State to push to.
 	attr_reader :state_class
 
+	##
+	# The data object to pass to the #state_class's constructor
+	attr_reader :data
+
 
 	### Apply the transition to the given +stack+.
 	def apply( stack )
-		state = self.state_class.new
+		state = self.state_class.new( self.data )
 
 		self.log.debug "pushing a new state: %p" % [ state ]
-		self.data = stack.last.on_pause( self.data ) if stack.last
+		stack.last.on_pause if stack.last
 		stack.push( state )
-		state.on_start( self.data )
+		state.on_start
 
 		return stack
 	end
diff --git a/lib/pushdown/transition/replace.rb b/lib/pushdown/transition/replace.rb
--- a/lib/pushdown/transition/replace.rb
+++ b/lib/pushdown/transition/replace.rb
@@ -10,9 +10,11 @@
 
 	### Create a transition that will Replace all the states on the current stack
 	### with an instance of the given +state_class+.
-	def initialize( name, state_class, *args )
-		super( name, *args )
+	def initialize( name, state_class, data=nil )
+		super( name )
+
 		@state_class = state_class
+		@data = data
 	end
 
 
@@ -24,18 +26,22 @@
 	# The State to replace the stack members with.
 	attr_reader :state_class
 
+	##
+	# The data object to pass to the #state_class's constructor
+	attr_reader :data
+
 
 	### Apply the transition to the given +stack+.
 	def apply( stack )
-		state = self.state_class.new
+		state = self.state_class.new( self.data )
 
 		self.log.debug "replacing current state with a new state: %p" % [ state ]
 		while ( old_state = stack.pop )
-			self.data = old_state.on_stop( self.data )
+			old_state.on_stop
 		end
 
 		stack.push( state )
-		state.on_start( self.data )
+		state.on_start
 
 		return stack
 	end
diff --git a/lib/pushdown/transition/switch.rb b/lib/pushdown/transition/switch.rb
--- a/lib/pushdown/transition/switch.rb
+++ b/lib/pushdown/transition/switch.rb
@@ -10,9 +10,11 @@
 
 	### Create a transition that will Switch the current State with an instance of
 	### the given +state_class+ on the stack.
-	def initialize( name, state_class, *args )
-		super( name, *args )
+	def initialize( name, state_class, data=nil )
+		super( name )
+
 		@state_class = state_class
+		@data = data
 	end
 
 
@@ -24,16 +26,23 @@
 	# The State to push to.
 	attr_reader :state_class
 
+	##
+	# The data object to pass to the #state_class's constructor
+	attr_reader :data
+
 
 	### Apply the transition to the given +stack+.
 	def apply( stack )
-		state = self.state_class.new
+		raise Pushdown::TransitionError, "can't switch on an empty stack" if stack.empty?
+
+		state = self.state_class.new( self.data )
 
 		self.log.debug "switching current state with a new state: %p" % [ state ]
 		old_state = stack.pop
-		self.data = old_state.on_stop( self.data )
+		old_state.on_stop if old_state
+
 		stack.push( state )
-		state.on_start( self.data )
+		state.on_start
 
 		return stack
 	end
diff --git a/pushdown.gemspec b/pushdown.gemspec
--- a/pushdown.gemspec
+++ b/pushdown.gemspec
@@ -1,18 +1,18 @@
 # -*- encoding: utf-8 -*-
-# stub: pushdown 0.2.0.pre.20210830140713 ruby lib
+# stub: pushdown 0.3.0.pre.20210901151908 ruby lib
 
 Gem::Specification.new do |s|
   s.name = "pushdown".freeze
-  s.version = "0.2.0.pre.20210830140713"
+  s.version = "0.3.0.pre.20210901151908"
 
   s.required_rubygems_version = Gem::Requirement.new("> 1.3.1".freeze) if s.respond_to? :required_rubygems_version=
   s.metadata = { "bug_tracker_uri" => "http://todo.sr.ht/~ged/Pushdown", "changelog_uri" => "http://deveiate.org/code/pushdown/History_md.html", "documentation_uri" => "http://deveiate.org/code/pushdown", "homepage_uri" => "http://hg.sr.ht/~ged/Pushdown", "source_uri" => "http://hg.sr.ht/~ged/Pushdown" } if s.respond_to? :metadata=
   s.require_paths = ["lib".freeze]
   s.authors = ["Michael Granger".freeze]
-  s.date = "2021-08-30"
+  s.date = "2021-09-01"
   s.description = "A pushdown automaton toolkit for Ruby. It&#39;s based on the State Manager from the Amethyst project.".freeze
   s.email = ["ged@faeriemud.org".freeze]
-  s.files = ["History.md".freeze, "LICENSE.txt".freeze, "README.md".freeze, "lib/pushdown.rb".freeze, "lib/pushdown/automaton.rb".freeze, "lib/pushdown/exceptions.rb".freeze, "lib/pushdown/state.rb".freeze, "lib/pushdown/transition.rb".freeze, "lib/pushdown/transition/pop.rb".freeze, "lib/pushdown/transition/push.rb".freeze, "lib/pushdown/transition/replace.rb".freeze, "lib/pushdown/transition/switch.rb".freeze, "spec/pushdown/automaton_spec.rb".freeze, "spec/pushdown/state_spec.rb".freeze, "spec/pushdown/transition/pop_spec.rb".freeze, "spec/pushdown/transition/push_spec.rb".freeze, "spec/pushdown/transition/replace_spec.rb".freeze, "spec/pushdown/transition/switch_spec.rb".freeze, "spec/pushdown/transition_spec.rb".freeze, "spec/pushdown_spec.rb".freeze, "spec/spec_helper.rb".freeze]
+  s.files = ["History.md".freeze, "LICENSE.txt".freeze, "README.md".freeze, "lib/pushdown.rb".freeze, "lib/pushdown/automaton.rb".freeze, "lib/pushdown/exceptions.rb".freeze, "lib/pushdown/spec_helpers.rb".freeze, "lib/pushdown/state.rb".freeze, "lib/pushdown/transition.rb".freeze, "lib/pushdown/transition/pop.rb".freeze, "lib/pushdown/transition/push.rb".freeze, "lib/pushdown/transition/replace.rb".freeze, "lib/pushdown/transition/switch.rb".freeze, "spec/pushdown/automaton_spec.rb".freeze, "spec/pushdown/spec_helpers_spec.rb".freeze, "spec/pushdown/state_spec.rb".freeze, "spec/pushdown/transition/pop_spec.rb".freeze, "spec/pushdown/transition/push_spec.rb".freeze, "spec/pushdown/transition/replace_spec.rb".freeze, "spec/pushdown/transition/switch_spec.rb".freeze, "spec/pushdown/transition_spec.rb".freeze, "spec/pushdown_spec.rb".freeze, "spec/spec_helper.rb".freeze]
   s.homepage = "http://hg.sr.ht/~ged/Pushdown".freeze
   s.licenses = ["BSD-3-Clause".freeze]
   s.rubygems_version = "3.1.6".freeze
diff --git a/spec/pushdown/automaton_spec.rb b/spec/pushdown/automaton_spec.rb
--- a/spec/pushdown/automaton_spec.rb
+++ b/spec/pushdown/automaton_spec.rb
@@ -137,7 +137,7 @@
 				return self.state_data ||= {}
 			end
 
-			starting_state.define_method( :on_start ) do |data|
+			starting_state.define_method( :on_start ) do
 				data[:starting_started] = true
 			end
 
diff --git a/spec/pushdown/state_spec.rb b/spec/pushdown/state_spec.rb
--- a/spec/pushdown/state_spec.rb
+++ b/spec/pushdown/state_spec.rb
@@ -55,25 +55,25 @@
 
 		it "has a default (no-op) callback for when it is added to the stack" do
 			instance = subclass.new
-			expect( instance.on_start(state_data) ).to be_nil
+			expect( instance.on_start ).to be_nil
 		end
 
 
 		it "has a default (no-op) callback for when it is removed from the stack" do
 			instance = subclass.new
-			expect( instance.on_stop(state_data) ).to be_nil
+			expect( instance.on_stop ).to be_nil
 		end
 
 
 		it "has a default (no-op) callback for when it is pushed down on the stack" do
 			instance = subclass.new
-			expect( instance.on_pause(state_data) ).to be_nil
+			expect( instance.on_pause ).to be_nil
 		end
 
 
 		it "has a default (no-op) callback for when the stack is popped and it becomes current again" do
 			instance = subclass.new
-			expect( instance.on_resume(state_data) ).to be_nil
+			expect( instance.on_resume ).to be_nil
 		end
 
 	end
@@ -93,13 +93,13 @@
 
 		it "has a default (no-op) interval callback for when it is current" do
 			instance = subclass.new
-			expect( instance.update(state_data) ).to be_nil
+			expect( instance.update ).to be_nil
 		end
 
 
 		it "has a default (no-op) interval callback for when it is on the stack" do
 			instance = subclass.new
-			expect( instance.shadow_update(state_data) ).to be_nil
+			expect( instance.shadow_update ).to be_nil
 		end
 
 	end
@@ -136,6 +136,18 @@
 			expect( result.data ).to be_nil
 		end
 
+
+		it "can create a transition it has declared that doesn't take a state class" do
+			subclass.transition_pop( :quit )
+			instance = subclass.new
+
+			automaton = automaton_class.new
+
+			result = instance.transition( :quit, automaton, :state )
+			expect( result ).to be_a( Pushdown::Transition::Pop )
+			expect( result.name ).to eq( :quit )
+		end
+
 	end
 
 end
diff --git a/spec/pushdown/transition/pop_spec.rb b/spec/pushdown/transition/pop_spec.rb
--- a/spec/pushdown/transition/pop_spec.rb
+++ b/spec/pushdown/transition/pop_spec.rb
@@ -17,8 +17,6 @@
 	let( :state_b ) { state_class.new }
 	let( :state_c ) { state_class.new }
 
-	let( :state_data ) { Object.new }
-
 	let( :stack ) do
 		return [ state_a, state_b, state_c ]
 	end
@@ -35,11 +33,10 @@
 
 
 	it "passes state data through the transition callbacks" do
-		transition = described_class.new( :pop_test, state_data )
+		transition = described_class.new( :pop_test )
 
-		expect( state_c ).to receive( :on_stop ).with( state_data ).
-			and_return( state_data ).once.ordered
-		expect( state_b ).to receive( :on_resume ).with( state_data ).once.ordered
+		expect( state_c ).to receive( :on_stop ).with( no_args ).once.ordered
+		expect( state_b ).to receive( :on_resume ).with( no_args ).once.ordered
 
 		transition.apply( stack )
 	end
diff --git a/spec/pushdown/transition/push_spec.rb b/spec/pushdown/transition/push_spec.rb
--- a/spec/pushdown/transition/push_spec.rb
+++ b/spec/pushdown/transition/push_spec.rb
@@ -16,18 +16,12 @@
 		Class.new( Pushdown::State )
 	end
 
-	let( :state_a ) { state_class.new }
-	let( :state_b ) { state_class.new }
-	let( :state_c ) { other_state_class.new }
-
-	let( :stack ) do
-		return [ state_a, state_b ]
-	end
-
 	let( :state_data ) { Object.new }
 
 
+
 	it "pushes a new state onto the stack when applied" do
+		stack = [ state_class.new, state_class.new ]
 		transition = described_class.new( :push_test, other_state_class )
 
 		new_stack = transition.apply( stack )
@@ -35,17 +29,31 @@
 		expect( new_stack ).to be_an( Array )
 		expect( new_stack.length ).to eq( 3 )
 		expect( new_stack.last ).to be_a( other_state_class )
+		expect( new_stack.last.data ).to be_nil
 	end
 
 
-	it "passes state data through the transition callbacks" do
-		transition = described_class.new( :push_test, other_state_class, state_data )
+	it "passes on state data to the new state if given" do
+		stack = []
+		transition = described_class.new( :push_test, state_class, state_data )
+
+		new_stack = transition.apply( stack )
+
+		expect( new_stack.last.data ).to be( state_data )
+	end
+
 
-		expect( state_b ).to receive( :on_pause ).
-			with( state_data ).once.and_return( state_data ).ordered
+	it "calls the transition callbacks of the former current state and the new state" do
+		new_state = instance_double( other_state_class )
+		stack = [
+			state_class.new,
+			state_class.new
+		]
+		transition = described_class.new( :push_test, other_state_class )
 
-		expect( other_state_class ).to receive( :new ).and_return( state_c )
-		expect( state_c ).to receive( :on_start ).with( state_data ).once.ordered
+		expect( stack.last ).to receive( :on_pause )
+		expect( other_state_class ).to receive( :new ).and_return( new_state )
+		expect( new_state ).to receive( :on_start )
 
 		transition.apply( stack )
 	end
diff --git a/spec/pushdown/transition/replace_spec.rb b/spec/pushdown/transition/replace_spec.rb
--- a/spec/pushdown/transition/replace_spec.rb
+++ b/spec/pushdown/transition/replace_spec.rb
@@ -28,6 +28,7 @@
 
 
 	it "replaces the current stack members with a single new instance of a state when applied" do
+		stack = [ state_class.new, state_class.new ]
 		transition = described_class.new( :replace_test, other_state_class )
 
 		new_stack = transition.apply( stack )
@@ -38,18 +39,31 @@
 	end
 
 
-	it "passes state data through the transition callbacks" do
-		transition = described_class.new( :replace_test, other_state_class, state_data )
+	it "passes on state data to the new state if given" do
+		stack = []
+		transition = described_class.new( :replace_test, state_class, state_data )
+
+		new_stack = transition.apply( stack )
+
+		expect( new_stack.last.data ).to be( state_data )
+	end
+
 
-		expect( state_c ).to receive( :on_stop ).
-			with( state_data ).once.and_return( state_data ).ordered
-		expect( state_b ).to receive( :on_stop ).
-			with( state_data ).once.and_return( state_data ).ordered
-		expect( state_a ).to receive( :on_stop ).
-			with( state_data ).once.and_return( state_data ).ordered
+	it "calls the transition callbacks of all the former states and the new state" do
+		new_state = instance_double( other_state_class )
+		stack = [
+			state_class.new,
+			state_class.new,
+			other_state_class.new
+		]
+		transition = described_class.new( :replace_test, other_state_class )
 
-		expect( other_state_class ).to receive( :new ).and_return( state_c )
-		expect( state_c ).to receive( :on_start ).with( state_data ).once.ordered
+		expect( stack[2] ).to receive( :on_stop ).ordered
+		expect( stack[1] ).to receive( :on_stop ).ordered
+		expect( stack[0] ).to receive( :on_stop ).ordered
+
+		expect( other_state_class ).to receive( :new ).and_return( new_state )
+		expect( new_state ).to receive( :on_start ).ordered
 
 		transition.apply( stack )
 	end
diff --git a/spec/pushdown/transition/switch_spec.rb b/spec/pushdown/transition/switch_spec.rb
--- a/spec/pushdown/transition/switch_spec.rb
+++ b/spec/pushdown/transition/switch_spec.rb
@@ -15,18 +15,12 @@
 		Class.new( Pushdown::State )
 	end
 
-	let( :state_a ) { state_class.new }
-	let( :state_b ) { state_class.new }
-	let( :state_c ) { other_state_class.new }
-
-	let( :stack ) do
-		return [ state_a, state_b ]
-	end
-
 	let( :state_data ) { Object.new }
 
 
-	it "pops the current state off the stack and adds a new state when applied" do
+
+	it "switches the current state the stack with a new one when applied" do
+		stack = [ state_class.new, state_class.new ]
 		transition = described_class.new( :switch_test, other_state_class )
 
 		new_stack = transition.apply( stack )
@@ -34,20 +28,44 @@
 		expect( new_stack ).to be_an( Array )
 		expect( new_stack.length ).to eq( 2 )
 		expect( new_stack.last ).to be_a( other_state_class )
+		expect( new_stack.last.data ).to be_nil
 	end
 
 
-	it "passes state data through the transition callbacks" do
-		transition = described_class.new( :switch_test, other_state_class, state_data )
+	it "passes on state data to the new state if given" do
+		stack = [ state_class.new ]
+		transition = described_class.new( :switch_test, state_class, state_data )
+
+		new_stack = transition.apply( stack )
+
+		expect( new_stack.last.data ).to be( state_data )
+	end
+
 
-		expect( state_b ).to receive( :on_stop ).
-			with( state_data ).once.and_return( state_data ).ordered
+	it "calls the transition callbacks of the former current state and the new state" do
+		new_state = instance_double( other_state_class )
+		stack = [
+			state_class.new,
+			state_class.new
+		]
+		transition = described_class.new( :switch_test, other_state_class )
 
-		expect( other_state_class ).to receive( :new ).and_return( state_c )
-		expect( state_c ).to receive( :on_start ).with( state_data ).once.ordered
+		expect( stack.last ).to receive( :on_stop )
+		expect( other_state_class ).to receive( :new ).and_return( new_state )
+		expect( new_state ).to receive( :on_start )
 
 		transition.apply( stack )
 	end
 
+
+	it "errors if applied to an empty stack" do
+		stack = []
+		transition = described_class.new( :switch_test, other_state_class )
+
+		expect {
+			transition.apply( stack )
+		}.to raise_error( Pushdown::TransitionError, /can't switch/i )
+	end
+
 end