From eefc9c2a894dc34a25fa4259c6669d781656972e Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 29 Dec 2019 18:43:40 -0600
Subject: [PATCH 1/6] Script: base verification for security

---
 Gemfile                    |   3 +-
 Gemfile.lock               |  12 +-
 lib/component.rb           |  21 +++
 lib/script.rb              | 130 ++++++++++++++++++
 lib/world.rb               |   1 +
 spec/lib/component_spec.rb |  42 ++++++
 spec/lib/script_spec.rb    | 271 +++++++++++++++++++++++++++++++++++++
 7 files changed, 475 insertions(+), 5 deletions(-)
 create mode 100644 lib/script.rb
 create mode 100644 spec/lib/script_spec.rb

diff --git a/Gemfile b/Gemfile
index 0827491..33255fc 100644
--- a/Gemfile
+++ b/Gemfile
@@ -9,6 +9,7 @@ gem 'pry'
 gem 'pry-remote'
 gem 'pry-rescue'
 gem 'pry-stack_explorer'
-gem 'malloc_trim'
+gem 'malloc_trim' # may be useful on linux
 gem 'memory_profiler'
 gem 'get_process_mem'
+gem 'parser'
diff --git a/Gemfile.lock b/Gemfile.lock
index d66b0fc..3409775 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,6 +1,7 @@
 GEM
   remote: https://rubygems.org/
   specs:
+    ast (2.4.0)
     binding_of_caller (0.8.0)
       debug_inspector (>= 0.0.1)
     coderay (1.1.2)
@@ -15,16 +16,18 @@ GEM
     interception (0.5)
     malloc_trim (0.1.0)
     memory_profiler (0.9.14)
-    method_source (0.9.0)
-    pry (0.11.3)
+    method_source (0.9.2)
+    parser (2.7.0.0)
+      ast (~> 2.4.0)
+    pry (0.12.2)
       coderay (~> 1.1.0)
       method_source (~> 0.9.0)
     pry-remote (0.1.8)
       pry (~> 0.9)
       slop (~> 3.0)
-    pry-rescue (1.4.5)
+    pry-rescue (1.5.0)
       interception (>= 0.5)
-      pry
+      pry (>= 0.12.0)
     pry-stack_explorer (0.4.9.3)
       binding_of_caller (>= 0.7)
       pry (>= 0.9.11)
@@ -53,6 +56,7 @@ DEPENDENCIES
   get_process_mem
   malloc_trim
   memory_profiler
+  parser
   pry
   pry-remote
   pry-rescue
diff --git a/lib/component.rb b/lib/component.rb
index 5608e4e..3471d70 100644
--- a/lib/component.rb
+++ b/lib/component.rb
@@ -257,4 +257,25 @@ def ==(other)
     self.class == other.class && to_h == other.to_h
   end
   alias eql? ==
+
+  # []
+  #
+  # Get the value for a field.  Slower method created to support Script.
+  def [](key)
+    key = key.to_sym unless key.is_a?(Symbol)
+    raise KeyError, "key not found: #{key}" unless
+        self.class.fields.include?(key)
+    send(key)
+  end
+
+  # []=
+  #
+  # Set the value for a field.  Slower than calling the setter directly.
+  # Implemented to support Script.
+  def []=(key, value)
+    key = key.to_sym unless key.is_a?(Symbol)
+    raise KeyError, "key not found: #{key}" unless
+        self.class.fields.include?(key)
+    send("#{key}=", value)
+  end
 end
diff --git a/lib/script.rb b/lib/script.rb
new file mode 100644
index 0000000..2eeea10
--- /dev/null
+++ b/lib/script.rb
@@ -0,0 +1,130 @@
+require 'forwardable'
+require 'parser/current'
+# opt-in to most recent AST format:
+Parser::Builders::Default.emit_lambda   = true
+Parser::Builders::Default.emit_index    = true
+
+# Script
+#
+# module for parsing world scripts
+class Script
+  class UnsafeScript < RuntimeError
+    def initialize(msg, root, node)
+      @root = root
+      @node = node
+      super(msg)
+    end
+  end
+
+  # Sandbox
+  #
+  # Sandbox in which scripts get evaluated.  All methods in the ScriptMethods
+  # array are available for the script to use.
+  module Sandbox
+    # variable names used when calling scripts; we fake like they are methods
+    # so that the safe-check knows they are permitted.
+    ScriptMethods = [ :entity, :actor ]
+
+    # pull in logging methods
+    extend Helpers::Logging
+    ScriptMethods.push(*::Helpers::Logging.public_instance_methods(false))
+    ScriptMethods.delete(:logger)   # they won't need to poke at logger
+
+    # explicitly pull in World::Helpers
+    extend ::World::Helpers
+    ScriptMethods.push(*::World::Helpers.public_instance_methods(false))
+
+    # Add other functionality from outside of the Sandbox in
+    class << self
+      extend Forwardable
+      def_delegators :Time, :now
+      def_delegators :Kernel, :rand
+    end
+    ScriptMethods.push(*singleton_class.public_instance_methods(false))
+  end
+
+  def initialize(script)
+    @script = script
+  end
+  attr_reader :ast
+
+  def safe!
+    @ast ||= Parser::CurrentRuby.parse(@script)
+    node_safe!(@ast)
+  end
+
+  # Based on our test-cases, these are the minimum nodes we need to permit in
+  # the AST.
+  #
+  # *** IMPORTANT ***
+  # Have a DAMN good reason to modify this list, and verify that all the tests
+  # pass before releasing changes.  This limited subset of functionality
+  # provides half the security for scripts.
+  #
+  # *** IMPORTANT ***
+  NodeTypeWhitelist = %i{
+      nil true false sym int float str array hash pair
+      lvasgn indexasgn masgn mlhs op_asgn and_asgn or_asgn
+      if case when while until irange erange begin
+      next break return block args arg or and
+      lvar index
+  }
+
+  # node_safe!
+  #
+  # Verify that a Parser::AST::Node is in our whitelist.  If it is not, raise
+  # an exception noting why the node is considered unsafe.
+  def node_safe!(node, lvars: {})
+    return unless node.is_a?(Parser::AST::Node)
+
+    case node.type
+    when :send
+      receiver, method = node.children
+      whitelist = receiver ? ReceiverMethodWhitelist : Sandbox::ScriptMethods
+
+      raise UnsafeScript
+          .new("method not permitted: #{method} in #{node}", @ast, node) unless
+              whitelist.include?(method)
+    else
+      raise UnsafeScript
+          .new("node type not in whitelist: #{node.type}", @ast, node) unless
+          NodeTypeWhitelist.include?(node.type)
+    end
+    node.children.each { |c| node_safe!(c) }
+  end
+
+  # Be VERY careful adding methods to this list.  If any type the script has
+  # access to implements these in a way that the caller can get access to
+  # `eval` or a generic Class or Module, security will be compromised.
+  #
+  # This list was generated from:
+  #   types = [ [], {}, '', 1, :Sym, true, false, nil, 1..1 ]
+  #   methods = types.map { |t| t.public_methods(false) }.flatten.sort.uniq
+  #
+  # Then going through the methods and manually verifying they do grant the
+  # script any additional access.  Someone else should review this.
+  ReceiverMethodWhitelist = %i{
+    ! % & | ^ + - * / == =~ === < > <= >= << >> [] []= first each map inject
+    nil?
+  }
+
+  # did this by hand, but fuuuuck that; let's do it each one as needed
+  %i{
+    % & * ** + +@ - -@ / < << <= <=> == === =~ > >= >> [] []= ^ abs all? any?
+    append at capitalize capitalize! ceil chomp chomp! chop chop! clear
+    collect collect! combination compact compact! count cycle delete delete!
+    delete_at delete_if downcase downcase! drop drop_while each each_char
+    each_index each_key each_line each_pair each_value empty? entries eql?
+    even? filter filter! find_index first flatten flatten! floor freeze gsub
+    gsub! has_key? has_value? include? indent indent! index insert inspect
+    join keep_if key key? keys last length lines ljust lstrip lstrip! map
+    map! match match? max member? merge merge! min modulo next next! nil?
+    none? odd? one? pop prepend push reject reject! replace reverse
+    reverse! reverse_each rindex rjust rotate rotate! round rstrip rstrip!
+    sample scan select select! shift shuffle shuffle! size slice slice! sort
+    sort! sort_by! split squeeze squeeze! step strip strip! sub sub! sum
+    take take_while times to_a to_ary to_f to_h to_hash to_i to_int to_s to_str
+    to_sym truncate union uniq uniq! unshift upcase upcase! upto value?
+    values values_at zip
+  }
+end
diff --git a/lib/world.rb b/lib/world.rb
index ddad566..0790faa 100644
--- a/lib/world.rb
+++ b/lib/world.rb
@@ -159,4 +159,5 @@ def update
 require_relative 'world/loader'
 require_relative 'system'
 require_relative 'command'
+require_relative 'script'
 World.extend(World::Helpers)
diff --git a/spec/lib/component_spec.rb b/spec/lib/component_spec.rb
index dd43e25..ba666d2 100644
--- a/spec/lib/component_spec.rb
+++ b/spec/lib/component_spec.rb
@@ -403,4 +403,46 @@
       expect(c.get_modified_fields).to eq({})
     end
   end
+
+  describe '#[](field)' do
+    let(:comp) do
+      Class.new(Component) { field :a }.new
+    end
+    context 'valid field name' do
+      it 'will call #field()' do
+        expect(comp).to receive(:a)
+        comp[:a]
+      end
+    end
+    context 'invalid field name' do
+      it 'will raise KeyError' do
+        expect { comp[:bad] }.to raise_error(KeyError)
+      end
+      it 'will not call #field()' do
+        expect(comp).to_not receive(:bad)
+        expect { comp[:bad] }.to raise_error(KeyError)
+      end
+    end
+  end
+
+  describe '#[]=(field, value)' do
+    let(:comp) do
+      Class.new(Component) { field :a }.new
+    end
+    context 'valid field name' do
+      it 'will call #field=(value)' do
+        expect(comp).to receive(:a=).with(:passed)
+        comp[:a] = :passed
+      end
+    end
+    context 'invalid field name' do
+      it 'will raise KeyError' do
+        expect { comp[:bad] = :failed }.to raise_error(KeyError)
+      end
+      it 'will not call #field=(value)' do
+        expect(comp).to_not receive(:bad=)
+        expect { comp[:bad] = :failed }.to raise_error(KeyError)
+      end
+    end
+  end
 end
diff --git a/spec/lib/script_spec.rb b/spec/lib/script_spec.rb
new file mode 100644
index 0000000..2eb22ff
--- /dev/null
+++ b/spec/lib/script_spec.rb
@@ -0,0 +1,271 @@
+describe Script do
+  describe '#safe!' do
+    shared_examples 'no error' do
+      it 'will not raise an error' do
+        expect { script.safe! }.to_not raise_error
+      end
+    end
+
+    shared_examples 'raise error' do
+      it 'will raise an UnsafeScript error' do
+        expect { script.safe! }.to raise_error(Script::UnsafeScript)
+      end
+    end
+
+    [
+        # base types
+        { desc: 'nil', code: 'nil', include: 'no error' },
+        { desc: 'true', code: 'true', include: 'no error' },
+        { desc: 'false', code: 'false', include: 'no error' },
+        { desc: 'Symbol', code: ':symbol', include: 'no error' },
+        { desc: 'Integer', code: '47', include: 'no error' },
+        { desc: 'Float', code: '47.0', include: 'no error' },
+        { desc: 'String', code: '"wolf"', include: 'no error' },
+        { desc: 'Array', code: '[]', include: 'no error' },
+        { desc: 'Hash', code: '{}', include: 'no error' },
+        { desc: 'inclusive Range', code: '1..10', include: 'no error' },
+        { desc: 'exclusive Range', code: '1...10', include: 'no error' },
+
+        # constants are not permitted; makes everything much easier to
+        # implement scripting safely.
+        { desc: 'constant World', code: 'World', include: 'raise error' },
+        { desc: 'constant World::Helpers',
+          code: 'World::Helpers',
+          include: 'raise error' },
+        { desc: 'constant File', code: 'File', include: 'raise error' },
+        { desc: 'constant Kernel', code: 'Kernel', include: 'raise error' },
+        { desc: 'constant Object', code: 'Object', include: 'raise error' },
+        { desc: 'constant Script', code: 'Script', include: 'raise error' },
+
+        # assignment
+        { desc: 'assign local var',
+          code: 'local = true',
+          include: 'no error' },
+        { desc: 'assign instance var',
+          code: '@instance_var = true',
+          include: 'raise error' },
+        { desc: 'assign class var',
+          code: '@@class_var = true',
+          include: 'raise error' },
+        { desc: 'assign constant',
+          code: 'Constant = true',
+          include: 'raise error' },
+        { desc: 'assign global variable',
+          code: '$global_var = true',
+          include: 'raise error' },
+
+        { desc: 'multiple assignment to local variables',
+          code: 'a,b = [1,2]',
+          include: 'no error' },
+        { desc: 'operation assignment to local var',
+          code: 'a += 1',
+          include: 'no error' },
+        { desc: 'logical-and operator assignment on local var',
+          code: 'a &&= 1',
+          include: 'no error' },
+        { desc: 'logical-or operator assignment on local var',
+          code: 'a ||= 1',
+          include: 'no error' },
+
+        { desc: 'assign array index',
+          code: '[][5] = true',
+          include: 'no error' },
+        { desc: 'access array index',
+          code: '[][5]',
+          include: 'no error' },
+
+        { desc: 'assign hash index',
+          code: '{}[5] = true',
+          include: 'no error' },
+        { desc: 'access hash index',
+          code: '{}[5]',
+          include: 'no error' },
+
+        { desc: 'accessing local variable',
+          code: 'a = true; a',
+          include: 'no error' },
+
+        # module & class creation/modification
+        { desc: 'module creation/modification',
+          code: 'module Meep; end',
+          include: 'raise error' },
+        { desc: 'class creation/modification',
+          code: 'class Meep; end',
+          include: 'raise error' },
+        { desc: 'singleton class',
+          include: 'raise error',
+          code: <<~CODE
+            class << self
+              def breakout
+                # other code
+              end
+            end
+          CODE
+        },
+
+        # method (un)definition
+        { desc: 'define instance method',
+          include: 'raise error',
+          code: 'def xxx; end' },
+        { desc: 'undefine instance method',
+          include: 'raise error',
+          code: 'undef :xxx' },
+        { desc: 'define singleton method',
+          include: 'raise error',
+          code: 'def self.xxx; end' },
+
+        # aliasing
+        { desc: 'method aliasing',
+          include: 'raise error',
+          code: 'alias xxx yyy' },
+
+        ## flow control
+        # if
+        { desc: 'if statement',
+          code: 'if true; true; end',
+          include: 'no error' },
+        { desc: 'tail if statement',
+          code: 'true if true',
+          include: 'no error' },
+        { desc: 'if-else statement',
+          code: 'if true; true; else; false; end',
+          include: 'no error' },
+        { desc: 'if-elsif statement',
+          code: 'if true; true; elsif true; false; end',
+          include: 'no error' },
+
+        # unless
+        { desc: 'unless statement',
+          code: 'unless true; true; end',
+          include: 'no error' },
+        { desc: 'tail unless statement',
+          code: 'true unless true',
+          include: 'no error' },
+        { desc: 'unless-else statement',
+          code: 'unless true; true; else; false; end',
+          include: 'no error' },
+
+        # case
+        { desc: 'case statement',
+          include: 'no error',
+          code: <<~CODE
+            case true
+            when true
+              3
+            when false
+              4
+            else
+              5
+            end
+          CODE
+        },
+
+        # while/until
+        { desc: 'tail while statement',
+          include: 'no error',
+          code: <<~CODE
+            while true
+              3
+            end
+          CODE
+        },
+        { desc: 'tail while statement',
+          include: 'no error',
+          code: '3 while true' },
+        { desc: 'tail until statement',
+          include: 'no error',
+          code: <<~CODE
+            until true
+              3
+            end
+          CODE
+        },
+        { desc: 'tail until statement',
+          include: 'no error',
+          code: '3 until true' },
+
+        # next, break, and continue
+        { desc: 'next', code: 'next', include: 'no error' },
+        { desc: 'break', code: 'break', include: 'no error' },
+        { desc: 'return', code: 'return', include: 'no error' },
+
+        # or, and, not
+        { desc: 'or', code: 'true or false', include: 'no error' },
+        { desc: 'and', code: 'true and false', include: 'no error' },
+        { desc: 'not', code: 'not false', include: 'no error' },
+
+        # sending
+        #
+        { desc: 'send whitelist command to receiver',
+          include: 'no error',
+          code: '[1,2,3].map' },
+        { desc: 'sending whitelist command with no receiver',
+          code: 'get_component("wolf", :damage)',
+          include: 'no error' },
+        { desc: 'sending whitelisted command a block',
+          include: 'no error',
+          code: '[].map { |v| v + 1 }'},
+
+        { desc: 'send non-whitelist command to receiver',
+          include: 'raise error',
+          code: 'a.eval("File")' },
+        { desc: 'sending non-whitelist command with no receiver',
+          code: 'eval("File")',
+          include: 'raise error' },
+
+        # regression prevention
+        { desc: 'Symbol#to_proc',
+          include: 'raise error',
+          code: ':send.to_proc.call(:eval, :puts, "hello world")' },
+        { desc: 'back-tick shell command',
+          include: 'raise error',
+          code: '`ls`' },
+        { desc: '%x{} shell command',
+          include: 'raise error',
+          code: '%x{ls}' },
+        { desc: 'system() command',
+          include: 'raise error',
+          code: 'system("ls")' },
+        { desc: 'exec() command',
+          include: 'raise error',
+          code: 'exec("ls")' },
+        { desc: 'File.open()',
+          include: 'raise error',
+          code: 'File.open("/etc/passwd")' },
+        { desc: 'eval',
+          include: 'raise error',
+          code: 'eval("File")' },
+        { desc: 'instance_eval',
+          include: 'raise error',
+          code: 'instance_eval("File")' },
+        { desc: 'send()',
+          include: 'raise error',
+          code: 'send(:eval, "File")' },
+        { desc: 'require()',
+          include: 'raise error',
+          code: 'require("pry")' },
+
+        # complex examples
+        { desc: 'teleporter enter',
+          include: 'no error',
+          code: <<~CODE
+            porter = get_component(entity, :teleporter) or return
+            port = get_component!(actor, :teleport)
+            port[:dest] = porter[:dest]
+            port[:at] = now + porter[:delay]
+          CODE
+        },
+        { desc: 'teleporter exit',
+          include: 'no error',
+          code: <<~CODE
+            remove_component(actor, :teleport)
+          CODE
+        },
+    ].each do |desc: nil, code: nil, include: nil|
+      context(desc) do
+        let(:script) { Script.new(code) }
+        include_examples(include)
+      end
+    end
+  end
+end

From 1aa98effcd4bf578e424aa9bc76c35c07dc92f58 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 29 Dec 2019 22:12:02 -0600
Subject: [PATCH 2/6] OnEnter/OnExitComponent, teleport script, Script

---
 data/world/base.yml              |  22 +++
 lib/components.rb                |  48 +++++
 lib/script.rb                    | 212 ++++++++++++--------
 lib/script/sandbox.rb            |  46 +++++
 lib/world.rb                     |   1 +
 lib/world/helpers.rb             | 329 ++++---------------------------
 lib/world/script_safe_helpers.rb | 289 +++++++++++++++++++++++++++
 spec/lib/script_spec.rb          | 161 +++++++--------
 spec/lib/world/helpers_spec.rb   |  31 +++
 9 files changed, 683 insertions(+), 456 deletions(-)
 create mode 100644 lib/script/sandbox.rb
 create mode 100644 lib/world/script_safe_helpers.rb

diff --git a/data/world/base.yml b/data/world/base.yml
index 71e38d2..48e11f6 100644
--- a/data/world/base.yml
+++ b/data/world/base.yml
@@ -113,3 +113,25 @@
   - viewable:
       short: a red rubber ball
       long: a red rubber ball is on the floor
+
+- id: base:act/teleporter
+  components:
+  - on_enter: base:script/teleporter/enter
+  - on_exit: base:script/teleporter/exit
+  - teleporter
+
+- id: base:script/teleporter/enter
+  components:
+  - script: !script |
+      unless porter = get_component(entity, :teleporter)
+        error "#{entity} missing teleporter component"
+        return
+      end
+      port   = get_component!(actor, :teleport)
+      port[:dest] = porter[:dest]
+      port[:at]   = now + porter[:delay]
+
+- id: base:script/teleporter/exit
+  components:
+  - script: !script |
+      remove_component(actor, :teleport)
diff --git a/lib/components.rb b/lib/components.rb
index 8818e6c..f54f9ee 100644
--- a/lib/components.rb
+++ b/lib/components.rb
@@ -127,6 +127,54 @@ class ConnectionComponent < Component
   field :buf, default: ''   # String of pending output
 end
 
+# Used to specify a script to run when an entity is moved into a
+# ContainerComponent.  Think walking into a room, or putting an item in a bag.
+# Technically it could even be when a mob picks up an item.
+class OnEnterComponent < Component
+  # due to compositing architecture, we're going to permit multiple triggers
+  not_unique
+
+  # This points at the entity id for the script that should be run when this
+  # entity is entered.
+  field :script
+end
+
+class OnExitComponent < Component
+  # due to compositing architecture, we're going to permit multiple triggers
+  not_unique
+
+  # This points at the entity id for the script that should be run when this
+  # entity is exited.
+  field :script
+end
+
+# This component holds configuration for a teleporter.  It is used by the
+# teleporter script.
+class TeleporterComponent < Component
+
+  # destination entity
+  field :dest
+
+  # delay before entity should be moved
+  field :delay, valid: proc { |v| v.is_a?(Integer) }, default: 10
+end
+
+# This component is added to an entity that will be teleported at a later time.
+# It is added by the teleporter script
+class TeleportComponent < Component
+  # destination entity
+  field :dest, valid: proc { |v| World.entity_exists?(v) }
+
+  # Time at which they should be teleported
+  field :at
+end
+
+# Component that holds a script
+class ScriptComponent < Component
+  # Script instance
+  field :script, clone: false
+end
+
 ### QUESTIONABLE COMPONENTS ###
 class AffectComponent < Component
   not_unique
diff --git a/lib/script.rb b/lib/script.rb
index 2eeea10..6b79dba 100644
--- a/lib/script.rb
+++ b/lib/script.rb
@@ -1,14 +1,90 @@
 require 'forwardable'
 require 'parser/current'
-# opt-in to most recent AST format:
+
+# Turn on some things for the parser
 Parser::Builders::Default.emit_lambda   = true
 Parser::Builders::Default.emit_index    = true
 
 # Script
 #
-# module for parsing world scripts
+# module for securely running scripts
+#
+# This class takes ruby code, verifies that it falls within the subset of what
+# we deem as safe code, and runs it.
+#
+# We take an explicit whitelist approach to all of the code in the script.
+# Starting with parsing the ruby code into an AST, we only allow the following:
+#   * use of basic datatypes:
+#     * Integer, Float, String, Array, Hash, Symbol, Range
+#   * assignment to local variables
+#   * flow control (if/elsif/else, unless, while, case, until)
+#   * logical conditions (or, and, and not)
+#   * white-list based method calls
+#
+# Some things that are not permitted:
+#   * accessing any Constant
+#   * defining classes, modules, methods
+#
+# There are three whitelists that dictate what the script can run.  You can
+# read more about each one at their definitions.
+#   NodeTypeWhitelist: Parser::AST::Node types that are permitted in the source
+#   InstanceMethodWhitelist: methods that may be called on instances
+#   Sandbox::ScriptMethods: any other methods that may be called (no instance)
+#
 class Script
+  # NodeTypeWhitelist
+  #
+  # This is the list of Parser::AST::Node types that are permitted within the
+  # script source code.  From the test cases (see rspec), this is the minimum
+  # subset needed to provide a functional scripting facility.
+  #
+  #                           *** IMPORTANT ***
+  # Have a good reason to modify this list, and verify that all the tests pass
+  # before releasing changes.  This limited subset of functionality provides
+  # half the security for scripts.
+  #                           *** IMPORTANT ***
+  #
+  NodeTypeWhitelist = %i{
+      nil true false sym int float str array hash pair dstr
+      lvasgn indexasgn masgn mlhs op_asgn and_asgn or_asgn
+      if case when while until irange erange begin
+      next break return block args arg or and not
+      lvar index
+  }
+
+  # InstanceMethodWhitelist
+  #
+  # This is the list of methods a script may call on **any** instance it has
+  # access to.  This list must be kept as small as possible to reduce the
+  # attack surface.
+  #
+  # Before you add anything to this list:
+  #   * Be absolutely certain it is needed
+  #   * Verify that the added method does not have unintended side-effects on
+  #     other instances
+  #     * Look at the added method for each of the base types permitted:
+  #       * Array, Hash, String, Integer, Float, Symbol, Range, true, false,
+  #         and nil
+  #       * Make sure none of them provide access to `eval` or `send`
+  #       * Make sure none of them allow Strings to be converted to code
+  #     * Look at **EVERY** type returned from every method in Script::Sandbox
+  #       for the same things
+  #       * Note that most methods are pulled from World::ScriptSafeHelpers
+  #   * After doing all of that, be certain you actually need the function
+  #
+  # DO NOT MODIFY this list if you're not comfortable stating that your change
+  # **cannot** be used to remotely execute code on the server.
+  InstanceMethodWhitelist = %i{
+    ! % & | ^ + - * / == =~ === < > <= >= << >> [] []=
+    first each map inject nil?
+  }
+
+  # UnsafeScript
+  #
+  # Exception we raise for untrusted elements in the script
   class UnsafeScript < RuntimeError
+    attr_reader :root, :node
+
     def initialize(msg, root, node)
       @root = root
       @node = node
@@ -16,59 +92,56 @@ def initialize(msg, root, node)
     end
   end
 
-  # Sandbox
+  # initialize
   #
-  # Sandbox in which scripts get evaluated.  All methods in the ScriptMethods
-  # array are available for the script to use.
-  module Sandbox
-    # variable names used when calling scripts; we fake like they are methods
-    # so that the safe-check knows they are permitted.
-    ScriptMethods = [ :entity, :actor ]
-
-    # pull in logging methods
-    extend Helpers::Logging
-    ScriptMethods.push(*::Helpers::Logging.public_instance_methods(false))
-    ScriptMethods.delete(:logger)   # they won't need to poke at logger
-
-    # explicitly pull in World::Helpers
-    extend ::World::Helpers
-    ScriptMethods.push(*::World::Helpers.public_instance_methods(false))
-
-    # Add other functionality from outside of the Sandbox in
-    class << self
-      extend Forwardable
-      def_delegators :Time, :now
-      def_delegators :Kernel, :rand
-    end
-    ScriptMethods.push(*singleton_class.public_instance_methods(false))
+  # Create a new script with the source provided
+  def initialize(script)
+    @source = script.clone.freeze
   end
 
-  def initialize(script)
-    @script = script
+  # Make yaml create a Script instance for `!script` tags
+  YAML.add_tag '!script', self
+
+  # initializer when loaded from yaml
+  def init_with(coder)
+    raise RuntimeError, "invalid script: %s " %
+        [ coder.send(coder.type).inspect ] unless coder.type == :scalar
+    @source = coder.scalar.to_s.freeze
+    safe!
   end
-  attr_reader :ast
 
-  def safe!
-    @ast ||= Parser::CurrentRuby.parse(@script)
-    node_safe!(@ast)
+  # encode the Script to yaml
+  def encode_with(coder)
+    coder.scalar = @source
   end
 
-  # Based on our test-cases, these are the minimum nodes we need to permit in
-  # the AST.
+  # safe!
   #
-  # *** IMPORTANT ***
-  # Have a DAMN good reason to modify this list, and verify that all the tests
-  # pass before releasing changes.  This limited subset of functionality
-  # provides half the security for scripts.
+  # Check to see if the script source is safe to be run
+  def safe!
+    ast = Parser::CurrentRuby.parse(@source)
+    node_safe!(ast)
+    true
+  end
+
+  # call
   #
-  # *** IMPORTANT ***
-  NodeTypeWhitelist = %i{
-      nil true false sym int float str array hash pair
-      lvasgn indexasgn masgn mlhs op_asgn and_asgn or_asgn
-      if case when while until irange erange begin
-      next break return block args arg or and
-      lvar index
-  }
+  # Call the script with provided arguments.
+  def call(entity: nil, actor: nil)
+    # XXX for the moment, we'll verify each time.  Later let's clean this up
+    # and only verify safe once, cache the result, and freeze the class.
+    safe!
+
+    # eval the code within the Sandbox, but first set the entity & actor
+    # provided.
+    Sandbox.instance_exec(@source) do |source|
+      entity = entity
+      actor = actor
+      eval(source)
+    end
+  end
+
+  private
 
   # node_safe!
   #
@@ -79,52 +152,25 @@ def node_safe!(node, lvars: {})
 
     case node.type
     when :send
-      receiver, method = node.children
-      whitelist = receiver ? ReceiverMethodWhitelist : Sandbox::ScriptMethods
 
+      # This is a call to a method.  If there's a receiver, we'll use the
+      # instance method whitelist, otherwise we'll use the list of scriptable
+      # methods from the sandbox.
+      receiver, method = node.children
+      whitelist = receiver ? InstanceMethodWhitelist : Sandbox::ScriptMethods
       raise UnsafeScript
           .new("method not permitted: #{method} in #{node}", @ast, node) unless
               whitelist.include?(method)
+
     else
+
+      # If the node type is not supported, raise an exception.
       raise UnsafeScript
           .new("node type not in whitelist: #{node.type}", @ast, node) unless
           NodeTypeWhitelist.include?(node.type)
     end
     node.children.each { |c| node_safe!(c) }
   end
-
-  # Be VERY careful adding methods to this list.  If any type the script has
-  # access to implements these in a way that the caller can get access to
-  # `eval` or a generic Class or Module, security will be compromised.
-  #
-  # This list was generated from:
-  #   types = [ [], {}, '', 1, :Sym, true, false, nil, 1..1 ]
-  #   methods = types.map { |t| t.public_methods(false) }.flatten.sort.uniq
-  #
-  # Then going through the methods and manually verifying they do grant the
-  # script any additional access.  Someone else should review this.
-  ReceiverMethodWhitelist = %i{
-    ! % & | ^ + - * / == =~ === < > <= >= << >> [] []= first each map inject
-    nil?
-  }
-
-  # did this by hand, but fuuuuck that; let's do it each one as needed
-  %i{
-    % & * ** + +@ - -@ / < << <= <=> == === =~ > >= >> [] []= ^ abs all? any?
-    append at capitalize capitalize! ceil chomp chomp! chop chop! clear
-    collect collect! combination compact compact! count cycle delete delete!
-    delete_at delete_if downcase downcase! drop drop_while each each_char
-    each_index each_key each_line each_pair each_value empty? entries eql?
-    even? filter filter! find_index first flatten flatten! floor freeze gsub
-    gsub! has_key? has_value? include? indent indent! index insert inspect
-    join keep_if key key? keys last length lines ljust lstrip lstrip! map
-    map! match match? max member? merge merge! min modulo next next! nil?
-    none? odd? one? pop prepend push reject reject! replace reverse
-    reverse! reverse_each rindex rjust rotate rotate! round rstrip rstrip!
-    sample scan select select! shift shuffle shuffle! size slice slice! sort
-    sort! sort_by! split squeeze squeeze! step strip strip! sub sub! sum
-    take take_while times to_a to_ary to_f to_h to_hash to_i to_int to_s to_str
-    to_sym truncate union uniq uniq! unshift upcase upcase! upto value?
-    values values_at zip
-  }
 end
+
+require_relative 'script/sandbox'
diff --git a/lib/script/sandbox.rb b/lib/script/sandbox.rb
new file mode 100644
index 0000000..666d72a
--- /dev/null
+++ b/lib/script/sandbox.rb
@@ -0,0 +1,46 @@
+# Sandbox
+#
+# Sandbox in which scripts get evaluated.  All methods in the ScriptMethods
+# array are available for the script to use.
+module Script::Sandbox
+
+  # ScriptMethods
+  #
+  # This is the list of methods the script is allowed to call in the sandbox.
+  # There is a second list of instance methods the script may call in
+  # Script::InstanceMethodWhitelist.
+  #
+  # Be **extremely** careful about what methods you add to the sandbox.
+  # Methods you add **may** be used to remotely exploit the server.
+  # DO NOT MODIFY this sandbox if you do not understand how a new method could
+  # result in security compromise.
+  #
+  # If you do modify this, be sure you run the test cases before committing.
+  # There are some generic tests in there to detect blatantly bad additions,
+  # BUT IN NO WAY IS IT COMPLETE.
+  #
+  # First things in this list are actually variables we get when the script is
+  # run.  We act like they're methods so that Script#safe! doesn't think
+  # they're unauthorized methods.
+  ScriptMethods = [ :entity, :actor ]
+
+  # Pull in the standard logging functions
+  extend Helpers::Logging
+  ScriptMethods.push(*::Helpers::Logging.public_instance_methods(false))
+  ScriptMethods.delete(:logger)   # but they don't need the logger method
+
+  # explicitly pull in World::ScriptSafeHelpers
+  extend ::World::ScriptSafeHelpers
+  ScriptMethods
+      .push(*::World::ScriptSafeHelpers.public_instance_methods(false))
+
+  # Scripts may not use constants, so we have to add helper methods by hand to
+  # expose some of the things we need.
+  class << self
+    extend Forwardable
+    def_delegators :Time, :now
+    def_delegators :Kernel, :rand
+  end
+  ScriptMethods.push(*singleton_class.public_instance_methods(false))
+end
+
diff --git a/lib/world.rb b/lib/world.rb
index 0790faa..0baad2f 100644
--- a/lib/world.rb
+++ b/lib/world.rb
@@ -155,6 +155,7 @@ def update
 end
 
 require_relative 'world/constants'
+require_relative 'world/script_safe_helpers'
 require_relative 'world/helpers'
 require_relative 'world/loader'
 require_relative 'system'
diff --git a/lib/world/helpers.rb b/lib/world/helpers.rb
index 9a4c085..95dfbd8 100644
--- a/lib/world/helpers.rb
+++ b/lib/world/helpers.rb
@@ -1,301 +1,10 @@
 require 'forwardable'
 require 'facets/string/indent'
 
+# World::Helpers
 module World::Helpers
   include Helpers::Logging
-  extend Forwardable
-  def_delegators :World, :create_entity, :destroy_entity, :add_component,
-      :remove_component, :get_component, :get_components, :get_component!
-
-  # raise an exception with a message, and all manner of extra data
-  #
-  # Arguments:
-  #   ++msg++ Message to include in the RuntimeError exception
-  #   ++data++ Additional context data for the exception; ex.data
-  #
-  # Return: None; exception raised
-  #
-  def fault(msg, *data)
-    ex = World::Fault.new(msg, *data)
-    ex.set_backtrace(caller)
-    raise(ex)
-  end
-
-  # Get the cardinal direction from the passage, or the first keyword
-  def exit_name(passage)
-    keywords = [ passage.get(:keywords, :words) ].flatten
-    (keywords & World::CARDINAL_DIRECTIONS).first or keywords.first
-  end
-
-  # place one entity inside another entity's contents
-  def move_entity(dest: nil, entity: nil)
-    container = get_component!(dest, :container)
-    location = get_component!(entity, :location)
-
-    if old = location.entity and src = get_component(old, :container)
-      debug "moving #{entity} from #{old} to #{dest}"
-      src.contents.delete(entity)
-    else
-      debug "moving #{entity} to #{dest}"
-    end
-
-    location.entity = dest
-    container.contents << entity
-  end
-
-  # get a/all entities from ++pool++ that have keywords that match our
-  # the provided ++keyword++
-  #
-  # Arguments:
-  #   ++buf++ String; "sword", "sharp-sword", "3.sword", "all.sword"
-  #   ++pool++ Entity ids
-  #
-  # Parameters:
-  #   ++multiple++ set to true if more than one match permitted
-  #
-  # Return:
-  #   when multiple: Array of matching entities in ++pool++
-  #   when not multiple: first entity in ++pool++ that matches
-  #
-  def match_keyword(buf, *pool, multiple: false)
-
-    fault "unparsable keyword; #{buf}" unless buf =~ /^(?:(all|\d+)\.)?(.*)$/
-    index = $1
-    keywords = $2.split('-').uniq
-
-    # ensure the user isn't using 'all.item' when the caller expects only a
-    # single item
-    raise Command::SyntaxError,
-        "'#{buf}' is not a valid target for this command" if
-            index == 'all' and !multiple
-
-    pool.flatten!
-
-    # if the user hasn't specified an index, or the caller hasn't specified
-    # that they want multiple matches, do the simple find here to grab and
-    # return the first match
-    if index.nil? and multiple != true
-      return pool.find do |entity|
-        comp = get_component(entity, :keywords) or next false
-        (comp.words & keywords).size == keywords.size
-      end
-    end
-
-    # Anything else requires us to have the full matches list
-    matches = pool.select do |entity|
-      comp = get_component(entity, :keywords) or next false
-      (comp.words & keywords).size == keywords.size
-    end
-
-    return matches if index.nil? or index == 'all'
-
-    index = index.to_i - 1
-    multiple ? [ matches[index] ].compact : matches[index]
-  end
-
-  # spawn_at
-  #
-  # Create a new instance of an entity from a base entity, and move it to dest.
-  #
-  # Arguments:
-  #   dest: container Entity to move entity to
-  #   base: base Entity to spawn
-  def spawn_at(dest: nil, base: nil)
-    raise ArgumentError, 'no dest' unless dest
-    raise ArgumentError, 'no base' unless base
-
-    entity = spawn(base: base, area: entity_area(dest))
-    move_entity(entity: entity, dest: dest)
-    entity
-  end
-
-  # spawn
-  #
-  # Create a new instance of an entity from a base entity
-  def spawn(base: [], area: nil)
-    entity = create_entity(base: base)
-    debug("spawning #{entity} from #{base}")
-    remove_component(entity, ViewExemptComponent)
-    get_component!(entity, MetadataComponent).area = area
-
-    if container = get_component(entity, ContainerComponent)
-      bases = container.contents.clone
-      container.contents = bases.map { |b| spawn_at(dest: entity, base: b ) }
-    end
-    if spawn_point = get_component(entity, SpawnPointComponent)
-      bases = spawn_point.list.clone
-      spawn_point.list = bases.map { |b| spawn(base: b, area: area) }
-    end
-
-    entity
-  end
-
-  # send_to_char
-  #
-  # Send output to entity if they have a connected ConnectionComponent
-  def send_to_char(char: nil, buf: nil)
-    conn_comp = get_component(char, ConnectionComponent) or return
-    return unless conn_comp.conn
-    conn_comp.buf << buf.to_s
-    nil
-  end
-
-  # entity_contents
-  #
-  # Array of entities within an entity's ContainerComponent
-  def entity_contents(entity)
-    comp = get_component(entity, ContainerComponent) or return []
-    comp.contents
-  end
-
-  # visible_contents
-  #
-  # Return the array of Entities within a Container Entity that are visibile to
-  # the actor.
-  def visible_contents(actor: nil, cont: nil)
-    raise ArgumentError, 'no actor' unless actor
-    raise ArgumentError, 'no container' unless cont
-
-    # XXX handle visibility checks at some point
-    comp = get_component(cont, ContainerComponent) or return []
-    comp.contents.select { |c| get_component(c, ViewableComponent) }
-  end
-
-  # entity_exits
-  #
-  # Get all the exits in a room; most likely you want to use visible_exits
-  # instead.
-  def entity_exits(room)
-    exits = get_component(room, ExitsComponent) or return []
-    exits.get_modified_fields.values
-  end
-
-  # visible_exits
-  #
-  # Return the array of exits visible to actor in room.
-  def visible_exits(actor: nil, room: nil)
-    raise ArgumentError, 'no actor' unless actor
-    raise ArgumentError, 'no room' unless room
-
-    # XXX handle visibility checks at some point
-
-    exits = get_component(room, ExitsComponent) or return []
-    World::CARDINAL_DIRECTIONS.map do |dir|
-      ex = exits.send(dir) or next
-      next if entity_closed?(ex) and entity_concealed?(ex)
-      ex
-    end.compact
-  end
-
-  # entity_exists?(entity)
-  #
-  # Returns true if entity exists
-  def entity_exists?(entity)
-    !!World.entities[entity]
-  end
-
-  # entity_components(entity)
-  #
-  # Returns array of Components for an entity.
-  #
-  # Note: Most likely you don't need this, and should be using get_view() or
-  # get_component() which are both faster.
-  def entity_components(entity)
-    World.entities[entity].compact
-  end
-
-  # entity_location(entity)
-  #
-  # Get the location for a given entity
-  def entity_location(entity)
-    loc = get_component(entity, LocationComponent) or return nil
-    loc.entity
-  end
-
-  # entity_desc
-  #
-  # Get a human-readable description for an entity
-  def entity_desc(entity)
-    desc = '%s: ' % entity
-    if words = entity_keywords(entity)
-      desc << "keywords='#{words}', "
-    else
-      desc << "keywords=nil, "
-    end
-
-    loc = entity_location(entity)
-    desc << 'loc=%s, ' % loc.inspect
-    components = em.entities[entity].compact.flatten
-        .map { |c| component_name(c) }
-    desc << 'comps=%s' % components.inspect
-    desc
-  end
-
-  # player_config
-  #
-  # Get a specific config value from a entity's PlayerConfigComponent
-  def player_config(player, option)
-    config = get_component(player, PlayerConfigComponent) or return nil
-    config.send(option)
-  end
-
-  # entity_keywords
-  #
-  # Get keywords for an entity
-  def entity_keywords(entity)
-    keywords = get_component(entity, KeywordsComponent) or return nil
-    words = keywords.words
-    words = [ words ] unless words.is_a?(Array)
-    words.join('-')
-  end
-
-  # component_name
-  #
-  # Given a Component instance or class, return the name
-  def component_name(arg)
-    arg = arg.class if arg.is_a?(Component)
-    arg.to_s.snakecase.sub(/_component$/, '').to_sym
-  end
-
-  # entity_area
-  #
-  # Get the area name from an entity id
-  def entity_area(entity)
-    meta = get_component(entity, :metadata) or return nil
-    meta.area
-  end
-
-  # entity_closed?
-  #
-  # Check if an entity has a ClosableComponent and is closed
-  def entity_closed?(entity)
-    closable = get_component(entity, ClosableComponent) or return false
-    !!closable.closed
-  end
-
-  # entity_locked?
-  #
-  # Check if an entity has a ClosableComponent and is locked
-  def entity_locked?(entity)
-    closable = get_component(entity, ClosableComponent) or return false
-    !!closable.locked
-  end
-
-  # entity_concealed?
-  #
-  # Check if an entity has a ConcealedComponent and it has not been revealed
-  def entity_concealed?(entity)
-    concealed = get_component(entity, :concealed) or return false
-    !concealed.revealed
-  end
-
-  # entity_short
-  #
-  # Get the short description for an entity
-  def entity_short(entity)
-    view = get_component(entity, ViewableComponent) or return nil
-    view.short
-  end
+  include World::ScriptSafeHelpers
 
   # player_prompt
   #
@@ -383,4 +92,38 @@ def load_entities(path, area: nil)
     loader.load(path: path, area: area)
     loader.finish
   end
+
+  # call_on_enter_scripts
+  #
+  # Call any on_enter scripts defined on location
+  def call_on_enter_scripts(location: nil, actor: nil)
+    fault 'no actor' unless actor
+    fault 'no location' unless location
+
+    get_components(location, :on_enter).each do |trig|
+      call_script(trig.script, entity: location, actor: actor)
+    end
+  end
+
+  # call_on_exit_scripts
+  #
+  # Call any on_enter scripts defined on location
+  def call_on_exit_scripts(location: nil, actor: nil)
+    fault 'no actor' unless actor
+    fault 'no location' unless location
+
+    get_components(location, :on_exit).each do |trig|
+      call_script(trig.script, entity: location, actor: actor)
+    end
+  end
+
+  # call_script
+  #
+  # Call the script on the supplied entity
+  def call_script(script_id, p={})
+    script = get_component(script_id, :script) or
+        fault "no script component in #{script_id}"
+    fault "#{script_id} script component is empty" unless script.script
+    script.script.call(p)
+  end
 end
diff --git a/lib/world/script_safe_helpers.rb b/lib/world/script_safe_helpers.rb
new file mode 100644
index 0000000..adddb07
--- /dev/null
+++ b/lib/world/script_safe_helpers.rb
@@ -0,0 +1,289 @@
+require 'forwardable'
+require 'facets/string/indent'
+
+# World::ScriptSafeHelpers
+#
+# This file contains helpers that are **SAFE** to use within scripts.  Every
+# method added here will be exposed to scripts.  As such, be certain you're not
+# implementing something that can open the server up to exploit.
+#
+# If you're not sure, add your helper to World::Helpers instead.
+#
+module World::ScriptSafeHelpers
+  extend Forwardable
+  def_delegators :World, :create_entity, :destroy_entity, :add_component,
+      :remove_component, :get_component, :get_components, :get_component!
+
+  # raise an exception with a message, and all manner of extra data
+  #
+  # Arguments:
+  #   ++msg++ Message to include in the RuntimeError exception
+  #   ++data++ Additional context data for the exception; ex.data
+  #
+  # Return: None; exception raised
+  #
+  def fault(msg, *data)
+    ex = World::Fault.new(msg, *data)
+    ex.set_backtrace(caller)
+    raise(ex)
+  end
+
+  # Get the cardinal direction from the passage, or the first keyword
+  def exit_name(passage)
+    keywords = [ passage.get(:keywords, :words) ].flatten
+    (keywords & World::CARDINAL_DIRECTIONS).first or keywords.first
+  end
+
+  # place one entity inside another entity's contents
+  def move_entity(dest: nil, entity: nil)
+    container = get_component!(dest, :container)
+    location = get_component!(entity, :location)
+
+    if old = location.entity and src = get_component(old, :container)
+      debug "moving #{entity} from #{old} to #{dest}"
+      call_on_exit_scripts(actor: entity, location: old)
+      src.contents.delete(entity)
+    else
+      debug "moving #{entity} to #{dest}"
+    end
+
+    location.entity = dest
+    container.contents << entity
+    call_on_enter_scripts(actor: entity, location: dest)
+  end
+
+  # get a/all entities from ++pool++ that have keywords that match our
+  # the provided ++keyword++
+  #
+  # Arguments:
+  #   ++buf++ String; "sword", "sharp-sword", "3.sword", "all.sword"
+  #   ++pool++ Entity ids
+  #
+  # Parameters:
+  #   ++multiple++ set to true if more than one match permitted
+  #
+  # Return:
+  #   when multiple: Array of matching entities in ++pool++
+  #   when not multiple: first entity in ++pool++ that matches
+  #
+  def match_keyword(buf, *pool, multiple: false)
+
+    fault "unparsable keyword; #{buf}" unless buf =~ /^(?:(all|\d+)\.)?(.*)$/
+    index = $1
+    keywords = $2.split('-').uniq
+
+    # ensure the user isn't using 'all.item' when the caller expects only a
+    # single item
+    raise Command::SyntaxError,
+        "'#{buf}' is not a valid target for this command" if
+            index == 'all' and !multiple
+
+    pool.flatten!
+
+    # if the user hasn't specified an index, or the caller hasn't specified
+    # that they want multiple matches, do the simple find here to grab and
+    # return the first match
+    if index.nil? and multiple != true
+      return pool.find do |entity|
+        comp = get_component(entity, :keywords) or next false
+        (comp.words & keywords).size == keywords.size
+      end
+    end
+
+    # Anything else requires us to have the full matches list
+    matches = pool.select do |entity|
+      comp = get_component(entity, :keywords) or next false
+      (comp.words & keywords).size == keywords.size
+    end
+
+    return matches if index.nil? or index == 'all'
+
+    index = index.to_i - 1
+    multiple ? [ matches[index] ].compact : matches[index]
+  end
+
+  # spawn_at
+  #
+  # Create a new instance of an entity from a base entity, and move it to dest.
+  #
+  # Arguments:
+  #   dest: container Entity to move entity to
+  #   base: base Entity to spawn
+  def spawn_at(dest: nil, base: nil)
+    raise ArgumentError, 'no dest' unless dest
+    raise ArgumentError, 'no base' unless base
+
+    entity = spawn(base: base, area: entity_area(dest))
+    move_entity(entity: entity, dest: dest)
+    entity
+  end
+
+  # spawn
+  #
+  # Create a new instance of an entity from a base entity
+  def spawn(base: [], area: nil)
+    entity = create_entity(base: base)
+    debug("spawning #{entity} from #{base}")
+    remove_component(entity, ViewExemptComponent)
+    get_component!(entity, MetadataComponent).area = area
+
+    if container = get_component(entity, ContainerComponent)
+      bases = container.contents.clone
+      container.contents = bases.map { |b| spawn_at(dest: entity, base: b ) }
+    end
+    if spawn_point = get_component(entity, SpawnPointComponent)
+      bases = spawn_point.list.clone
+      spawn_point.list = bases.map { |b| spawn(base: b, area: area) }
+    end
+
+    entity
+  end
+
+  # send_to_char
+  #
+  # Send output to entity if they have a connected ConnectionComponent
+  def send_to_char(char: nil, buf: nil)
+    conn_comp = get_component(char, ConnectionComponent) or return
+    return unless conn_comp.conn
+    conn_comp.buf << buf.to_s
+    nil
+  end
+
+  # entity_contents
+  #
+  # Array of entities within an entity's ContainerComponent
+  def entity_contents(entity)
+    comp = get_component(entity, ContainerComponent) or return []
+    comp.contents
+  end
+
+  # visible_contents
+  #
+  # Return the array of Entities within a Container Entity that are visibile to
+  # the actor.
+  def visible_contents(actor: nil, cont: nil)
+    raise ArgumentError, 'no actor' unless actor
+    raise ArgumentError, 'no container' unless cont
+
+    # XXX handle visibility checks at some point
+    comp = get_component(cont, ContainerComponent) or return []
+    comp.contents.select { |c| get_component(c, ViewableComponent) }
+  end
+
+  # entity_exits
+  #
+  # Get all the exits in a room; most likely you want to use visible_exits
+  # instead.
+  def entity_exits(room)
+    exits = get_component(room, ExitsComponent) or return []
+    exits.get_modified_fields.values
+  end
+
+  # visible_exits
+  #
+  # Return the array of exits visible to actor in room.
+  def visible_exits(actor: nil, room: nil)
+    raise ArgumentError, 'no actor' unless actor
+    raise ArgumentError, 'no room' unless room
+
+    # XXX handle visibility checks at some point
+
+    exits = get_component(room, ExitsComponent) or return []
+    World::CARDINAL_DIRECTIONS.map do |dir|
+      ex = exits.send(dir) or next
+      next if entity_closed?(ex) and entity_concealed?(ex)
+      ex
+    end.compact
+  end
+
+  # entity_exists?(entity)
+  #
+  # Returns true if entity exists
+  def entity_exists?(entity)
+    !!World.entities[entity]
+  end
+
+  # entity_components(entity)
+  #
+  # Returns array of Components for an entity.
+  #
+  # Note: Most likely you don't need this, and should be using get_view() or
+  # get_component() which are both faster.
+  def entity_components(entity)
+    World.entities[entity].compact
+  end
+
+  # entity_location(entity)
+  #
+  # Get the location for a given entity
+  def entity_location(entity)
+    loc = get_component(entity, LocationComponent) or return nil
+    loc.entity
+  end
+
+  # player_config
+  #
+  # Get a specific config value from a entity's PlayerConfigComponent
+  def player_config(player, option)
+    config = get_component(player, PlayerConfigComponent) or return nil
+    config.send(option)
+  end
+
+  # entity_keywords
+  #
+  # Get keywords for an entity
+  def entity_keywords(entity)
+    keywords = get_component(entity, KeywordsComponent) or return nil
+    words = keywords.words
+    words = [ words ] unless words.is_a?(Array)
+    words.join('-')
+  end
+
+  # component_name
+  #
+  # Given a Component instance or class, return the name
+  def component_name(arg)
+    arg = arg.class if arg.is_a?(Component)
+    arg.to_s.snakecase.sub(/_component$/, '').to_sym
+  end
+
+  # entity_area
+  #
+  # Get the area name from an entity id
+  def entity_area(entity)
+    meta = get_component(entity, :metadata) or return nil
+    meta.area
+  end
+
+  # entity_closed?
+  #
+  # Check if an entity has a ClosableComponent and is closed
+  def entity_closed?(entity)
+    closable = get_component(entity, ClosableComponent) or return false
+    !!closable.closed
+  end
+
+  # entity_locked?
+  #
+  # Check if an entity has a ClosableComponent and is locked
+  def entity_locked?(entity)
+    closable = get_component(entity, ClosableComponent) or return false
+    !!closable.locked
+  end
+
+  # entity_concealed?
+  #
+  # Check if an entity has a ConcealedComponent and it has not been revealed
+  def entity_concealed?(entity)
+    concealed = get_component(entity, :concealed) or return false
+    !concealed.revealed
+  end
+
+  # entity_short
+  #
+  # Get the short description for an entity
+  def entity_short(entity)
+    view = get_component(entity, ViewableComponent) or return nil
+    view.short
+  end
+end
diff --git a/spec/lib/script_spec.rb b/spec/lib/script_spec.rb
index 2eb22ff..718166e 100644
--- a/spec/lib/script_spec.rb
+++ b/spec/lib/script_spec.rb
@@ -14,86 +14,90 @@
 
     [
         # base types
-        { desc: 'nil', code: 'nil', include: 'no error' },
-        { desc: 'true', code: 'true', include: 'no error' },
-        { desc: 'false', code: 'false', include: 'no error' },
-        { desc: 'Symbol', code: ':symbol', include: 'no error' },
-        { desc: 'Integer', code: '47', include: 'no error' },
-        { desc: 'Float', code: '47.0', include: 'no error' },
-        { desc: 'String', code: '"wolf"', include: 'no error' },
-        { desc: 'Array', code: '[]', include: 'no error' },
-        { desc: 'Hash', code: '{}', include: 'no error' },
-        { desc: 'inclusive Range', code: '1..10', include: 'no error' },
-        { desc: 'exclusive Range', code: '1...10', include: 'no error' },
+        { desc: 'nil', code: 'nil', expect: 'no error' },
+        { desc: 'true', code: 'true', expect: 'no error' },
+        { desc: 'false', code: 'false', expect: 'no error' },
+        { desc: 'Symbol', code: ':symbol', expect: 'no error' },
+        { desc: 'Integer', code: '47', expect: 'no error' },
+        { desc: 'Float', code: '47.0', expect: 'no error' },
+        { desc: 'String', code: '"wolf"', expect: 'no error' },
+        { desc: 'Array', code: '[]', expect: 'no error' },
+        { desc: 'Hash', code: '{}', expect: 'no error' },
+        { desc: 'inclusive Range', code: '1..10', expect: 'no error' },
+        { desc: 'exclusive Range', code: '1...10', expect: 'no error' },
+        { desc: 'interpolated String', expect: 'no error',
+          code: 'x = 3; "wolf #{x}"' },
+        { desc: 'format String', expect: 'no error',
+          code: 'x = 3; "wolf %d" % x' },
 
         # constants are not permitted; makes everything much easier to
         # implement scripting safely.
-        { desc: 'constant World', code: 'World', include: 'raise error' },
+        { desc: 'constant World', code: 'World', expect: 'raise error' },
         { desc: 'constant World::Helpers',
           code: 'World::Helpers',
-          include: 'raise error' },
-        { desc: 'constant File', code: 'File', include: 'raise error' },
-        { desc: 'constant Kernel', code: 'Kernel', include: 'raise error' },
-        { desc: 'constant Object', code: 'Object', include: 'raise error' },
-        { desc: 'constant Script', code: 'Script', include: 'raise error' },
+          expect: 'raise error' },
+        { desc: 'constant File', code: 'File', expect: 'raise error' },
+        { desc: 'constant Kernel', code: 'Kernel', expect: 'raise error' },
+        { desc: 'constant Object', code: 'Object', expect: 'raise error' },
+        { desc: 'constant Script', code: 'Script', expect: 'raise error' },
 
         # assignment
         { desc: 'assign local var',
           code: 'local = true',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'assign instance var',
           code: '@instance_var = true',
-          include: 'raise error' },
+          expect: 'raise error' },
         { desc: 'assign class var',
           code: '@@class_var = true',
-          include: 'raise error' },
+          expect: 'raise error' },
         { desc: 'assign constant',
           code: 'Constant = true',
-          include: 'raise error' },
+          expect: 'raise error' },
         { desc: 'assign global variable',
           code: '$global_var = true',
-          include: 'raise error' },
+          expect: 'raise error' },
 
         { desc: 'multiple assignment to local variables',
           code: 'a,b = [1,2]',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'operation assignment to local var',
           code: 'a += 1',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'logical-and operator assignment on local var',
           code: 'a &&= 1',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'logical-or operator assignment on local var',
           code: 'a ||= 1',
-          include: 'no error' },
+          expect: 'no error' },
 
         { desc: 'assign array index',
           code: '[][5] = true',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'access array index',
           code: '[][5]',
-          include: 'no error' },
+          expect: 'no error' },
 
         { desc: 'assign hash index',
           code: '{}[5] = true',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'access hash index',
           code: '{}[5]',
-          include: 'no error' },
+          expect: 'no error' },
 
         { desc: 'accessing local variable',
           code: 'a = true; a',
-          include: 'no error' },
+          expect: 'no error' },
 
         # module & class creation/modification
         { desc: 'module creation/modification',
           code: 'module Meep; end',
-          include: 'raise error' },
+          expect: 'raise error' },
         { desc: 'class creation/modification',
           code: 'class Meep; end',
-          include: 'raise error' },
+          expect: 'raise error' },
         { desc: 'singleton class',
-          include: 'raise error',
+          expect: 'raise error',
           code: <<~CODE
             class << self
               def breakout
@@ -105,49 +109,49 @@ def breakout
 
         # method (un)definition
         { desc: 'define instance method',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'def xxx; end' },
         { desc: 'undefine instance method',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'undef :xxx' },
         { desc: 'define singleton method',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'def self.xxx; end' },
 
         # aliasing
         { desc: 'method aliasing',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'alias xxx yyy' },
 
         ## flow control
         # if
         { desc: 'if statement',
           code: 'if true; true; end',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'tail if statement',
           code: 'true if true',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'if-else statement',
           code: 'if true; true; else; false; end',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'if-elsif statement',
           code: 'if true; true; elsif true; false; end',
-          include: 'no error' },
+          expect: 'no error' },
 
         # unless
         { desc: 'unless statement',
           code: 'unless true; true; end',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'tail unless statement',
           code: 'true unless true',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'unless-else statement',
           code: 'unless true; true; else; false; end',
-          include: 'no error' },
+          expect: 'no error' },
 
         # case
         { desc: 'case statement',
-          include: 'no error',
+          expect: 'no error',
           code: <<~CODE
             case true
             when true
@@ -162,7 +166,7 @@ def breakout
 
         # while/until
         { desc: 'tail while statement',
-          include: 'no error',
+          expect: 'no error',
           code: <<~CODE
             while true
               3
@@ -170,10 +174,10 @@ def breakout
           CODE
         },
         { desc: 'tail while statement',
-          include: 'no error',
+          expect: 'no error',
           code: '3 while true' },
         { desc: 'tail until statement',
-          include: 'no error',
+          expect: 'no error',
           code: <<~CODE
             until true
               3
@@ -181,73 +185,73 @@ def breakout
           CODE
         },
         { desc: 'tail until statement',
-          include: 'no error',
+          expect: 'no error',
           code: '3 until true' },
 
         # next, break, and continue
-        { desc: 'next', code: 'next', include: 'no error' },
-        { desc: 'break', code: 'break', include: 'no error' },
-        { desc: 'return', code: 'return', include: 'no error' },
+        { desc: 'next', code: 'next', expect: 'no error' },
+        { desc: 'break', code: 'break', expect: 'no error' },
+        { desc: 'return', code: 'return', expect: 'no error' },
 
         # or, and, not
-        { desc: 'or', code: 'true or false', include: 'no error' },
-        { desc: 'and', code: 'true and false', include: 'no error' },
-        { desc: 'not', code: 'not false', include: 'no error' },
+        { desc: 'or', code: 'true or false', expect: 'no error' },
+        { desc: 'and', code: 'true and false', expect: 'no error' },
+        { desc: 'not', code: 'not false', expect: 'no error' },
 
         # sending
         #
         { desc: 'send whitelist command to receiver',
-          include: 'no error',
+          expect: 'no error',
           code: '[1,2,3].map' },
         { desc: 'sending whitelist command with no receiver',
           code: 'get_component("wolf", :damage)',
-          include: 'no error' },
+          expect: 'no error' },
         { desc: 'sending whitelisted command a block',
-          include: 'no error',
+          expect: 'no error',
           code: '[].map { |v| v + 1 }'},
 
         { desc: 'send non-whitelist command to receiver',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'a.eval("File")' },
         { desc: 'sending non-whitelist command with no receiver',
           code: 'eval("File")',
-          include: 'raise error' },
+          expect: 'raise error' },
 
         # regression prevention
         { desc: 'Symbol#to_proc',
-          include: 'raise error',
+          expect: 'raise error',
           code: ':send.to_proc.call(:eval, :puts, "hello world")' },
         { desc: 'back-tick shell command',
-          include: 'raise error',
+          expect: 'raise error',
           code: '`ls`' },
         { desc: '%x{} shell command',
-          include: 'raise error',
+          expect: 'raise error',
           code: '%x{ls}' },
         { desc: 'system() command',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'system("ls")' },
         { desc: 'exec() command',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'exec("ls")' },
         { desc: 'File.open()',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'File.open("/etc/passwd")' },
         { desc: 'eval',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'eval("File")' },
         { desc: 'instance_eval',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'instance_eval("File")' },
         { desc: 'send()',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'send(:eval, "File")' },
         { desc: 'require()',
-          include: 'raise error',
+          expect: 'raise error',
           code: 'require("pry")' },
 
-        # complex examples
+        # teleporter examples
         { desc: 'teleporter enter',
-          include: 'no error',
+          expect: 'no error',
           code: <<~CODE
             porter = get_component(entity, :teleporter) or return
             port = get_component!(actor, :teleport)
@@ -256,15 +260,12 @@ def breakout
           CODE
         },
         { desc: 'teleporter exit',
-          include: 'no error',
-          code: <<~CODE
-            remove_component(actor, :teleport)
-          CODE
-        },
-    ].each do |desc: nil, code: nil, include: nil|
+          expect: 'no error',
+          code: 'remove_component(actor, :teleport)' },
+    ].each do |desc: nil, code: nil, expect: nil|
       context(desc) do
         let(:script) { Script.new(code) }
-        include_examples(include)
+        include_examples(expect)
       end
     end
   end
diff --git a/spec/lib/world/helpers_spec.rb b/spec/lib/world/helpers_spec.rb
index e433019..be57ef6 100644
--- a/spec/lib/world/helpers_spec.rb
+++ b/spec/lib/world/helpers_spec.rb
@@ -5,6 +5,7 @@
 describe World::Helpers do
   include World::Helpers
   before(:all) { load_test_world }
+  let(:leo) { 'spec:mob/leonidas' }
 
   describe '.send_to_char(char: nil, buf: nil)' do
     context 'char has no ConnectionComponent' do
@@ -148,4 +149,34 @@ def entity_snapshot(entity)
       end
     end
   end
+
+  context '.move_entity' do
+    let(:script) { Script.new('true') }
+    let(:script_entity) do
+      create_entity(components: ScriptComponent.new(script: script))
+    end
+    let(:dest) { create_entity(base: 'base:room') }
+
+    context 'when dest has on_enter script' do
+      before(:each) do
+        add_component(dest, OnEnterComponent.new(script: script_entity))
+      end
+
+      it 'will call on_enter script' do
+        expect(script).to receive(:call).with(entity: dest, actor: leo)
+        move_entity(entity: leo, dest: dest)
+      end
+    end
+    context 'when entity location has on_exit script' do
+      before(:each) do
+        add_component(dest, OnExitComponent.new(script: script_entity))
+      end
+
+      it 'will call on_exit script' do
+        move_entity(entity: leo, dest: dest)
+        expect(script).to receive(:call).with(entity: dest, actor: leo)
+        move_entity(entity: leo, dest: create_entity)
+      end
+    end
+  end
 end

From 2c475fb9dad49f0683efb11890a3970c47da0471 Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Sun, 29 Dec 2019 22:55:41 -0600
Subject: [PATCH 3/6] added on_enter script in limbo

---
 data/world/limbo/rooms.yml       | 8 ++++++++
 lib/world/script_safe_helpers.rb | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/data/world/limbo/rooms.yml b/data/world/limbo/rooms.yml
index c5cc280..fbfa072 100644
--- a/data/world/limbo/rooms.yml
+++ b/data/world/limbo/rooms.yml
@@ -14,6 +14,7 @@
       list:
       - limbo:spawn/mob/puff
       - limbo:spawn/obj/limbo-chest
+  - on_enter: limbo:script/balls
 
 - base: base:exit
   link: base:room/void.exits.up
@@ -73,3 +74,10 @@
   components:
   - spawn:
       entity: limbo:mob/puff
+
+- id: limbo:script/balls
+  components:
+  - script: !script |
+      send_to_char(char: actor,
+          buf: "A small &Rred&0 ball appears in your hand!")
+      spawn_at(dest: actor, base: 'base:obj/junk/ball')
diff --git a/lib/world/script_safe_helpers.rb b/lib/world/script_safe_helpers.rb
index adddb07..56445aa 100644
--- a/lib/world/script_safe_helpers.rb
+++ b/lib/world/script_safe_helpers.rb
@@ -41,7 +41,7 @@ def move_entity(dest: nil, entity: nil)
 
     if old = location.entity and src = get_component(old, :container)
       debug "moving #{entity} from #{old} to #{dest}"
-      call_on_exit_scripts(actor: entity, location: old)
+      World.call_on_exit_scripts(actor: entity, location: old)
       src.contents.delete(entity)
     else
       debug "moving #{entity} to #{dest}"
@@ -49,7 +49,7 @@ def move_entity(dest: nil, entity: nil)
 
     location.entity = dest
     container.contents << entity
-    call_on_enter_scripts(actor: entity, location: dest)
+    World.call_on_enter_scripts(actor: entity, location: dest)
   end
 
   # get a/all entities from ++pool++ that have keywords that match our

From 201145b53868541e41f13c72a8afcd253fdee93a Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 30 Dec 2019 20:44:34 -0600
Subject: [PATCH 4/6] Script done; Hooks; enter/exit hooks; teleporter

* Cleanup of Script & expanded testing (closes #29)
* Implemented generic hook architecture (#32)
    * Implemented specific container enter/exit hooks
* Fixed ordering of enter/exit hooks with movement/look (fixes #31)
* Cleanup of teleporter script + tests
---
 Gemfile                          |  25 +++---
 Gemfile.lock                     |   2 -
 data/world/base.yml              |  20 +++--
 data/world/limbo/rooms.yml       |   7 +-
 lib/command/movement.rb          |   3 +-
 lib/components.rb                |  60 +++++++++----
 lib/script.rb                    | 124 +++++++++++++++++++-------
 lib/script/sandbox.rb            |   2 +-
 lib/system/connections.rb        |   4 -
 lib/world/helpers.rb             |  26 +++---
 lib/world/script_safe_helpers.rb |  52 ++++++++---
 spec/lib/script_spec.rb          | 147 ++++++++++++++++++++++---------
 spec/lib/world/helpers_spec.rb   | 104 +++++++++++++++++++---
 spec/script/teleporter_spec.rb   |  41 +++++++++
 14 files changed, 465 insertions(+), 152 deletions(-)
 create mode 100644 spec/script/teleporter_spec.rb

diff --git a/Gemfile b/Gemfile
index 33255fc..31a04ca 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,14 +2,19 @@ source 'https://rubygems.org'
 
 gem 'eventmachine'
 gem 'colorize'
-gem 'rspec'
-gem 'rspec-mocks'
 gem 'facets'
-gem 'pry'
-gem 'pry-remote'
-gem 'pry-rescue'
-gem 'pry-stack_explorer'
-gem 'malloc_trim' # may be useful on linux
-gem 'memory_profiler'
-gem 'get_process_mem'
-gem 'parser'
+gem 'parser'    # needed for lib/script.rb
+
+group :test do
+  gem 'rspec'
+  gem 'rspec-mocks'
+end
+
+group :test, :development do
+  gem 'pry'
+  gem 'pry-remote'
+  gem 'pry-rescue'
+  gem 'pry-stack_explorer'
+  gem 'memory_profiler'
+  gem 'get_process_mem'
+end
diff --git a/Gemfile.lock b/Gemfile.lock
index 3409775..e1c7ee3 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -14,7 +14,6 @@ GEM
     get_process_mem (0.2.5)
       ffi (~> 1.0)
     interception (0.5)
-    malloc_trim (0.1.0)
     memory_profiler (0.9.14)
     method_source (0.9.2)
     parser (2.7.0.0)
@@ -54,7 +53,6 @@ DEPENDENCIES
   eventmachine
   facets
   get_process_mem
-  malloc_trim
   memory_profiler
   parser
   pry
diff --git a/data/world/base.yml b/data/world/base.yml
index 48e11f6..f3bffb5 100644
--- a/data/world/base.yml
+++ b/data/world/base.yml
@@ -116,22 +116,28 @@
 
 - id: base:act/teleporter
   components:
-  - on_enter: base:script/teleporter/enter
-  - on_exit: base:script/teleporter/exit
+  - hook:
+      event: :on_enter
+      script: base:script/teleporter/enter
+  - hook:
+      event: :on_exit
+      script: base:script/teleporter/exit
   - teleporter
 
 - id: base:script/teleporter/enter
   components:
   - script: !script |
-      unless porter = get_component(entity, :teleporter)
-        error "#{entity} missing teleporter component"
+      here = args[:here]
+      unless porter = get_component(here, :teleporter)
+        error "#{here} missing teleporter component"
         return
       end
-      port   = get_component!(actor, :teleport)
+      entity = args[:entity]
+      port = get_component!(entity, :teleport)
       port[:dest] = porter[:dest]
-      port[:at]   = now + porter[:delay]
+      port[:at] = now + porter[:delay]
 
 - id: base:script/teleporter/exit
   components:
   - script: !script |
-      remove_component(actor, :teleport)
+      remove_component(args[:entity], :teleport)
diff --git a/data/world/limbo/rooms.yml b/data/world/limbo/rooms.yml
index fbfa072..25d8645 100644
--- a/data/world/limbo/rooms.yml
+++ b/data/world/limbo/rooms.yml
@@ -14,7 +14,9 @@
       list:
       - limbo:spawn/mob/puff
       - limbo:spawn/obj/limbo-chest
-  - on_enter: limbo:script/balls
+  - hook:
+      event: :on_enter
+      script: limbo:script/balls
 
 - base: base:exit
   link: base:room/void.exits.up
@@ -78,6 +80,7 @@
 - id: limbo:script/balls
   components:
   - script: !script |
+      actor = args[:entity]
       send_to_char(char: actor,
-          buf: "A small &Rred&0 ball appears in your hand!")
+          buf: "A small &Rred&0 ball appears in your hand!\n")
       spawn_at(dest: actor, base: 'base:obj/junk/ball')
diff --git a/lib/command/movement.rb b/lib/command/movement.rb
index 6731493..551a7da 100644
--- a/lib/command/movement.rb
+++ b/lib/command/movement.rb
@@ -27,8 +27,7 @@ def traverse_passage(actor, name)
       else
         dest = get_component(passage, :destination) or
             fault "passage #{passage} has no destination!"
-        move_entity(entity: actor, dest: dest.entity)
-        Command::Look.show_room(actor, dest.entity)
+        move_entity(entity: actor, dest: dest.entity, look: true)
       end
     end
   end
diff --git a/lib/components.rb b/lib/components.rb
index f54f9ee..17395fb 100644
--- a/lib/components.rb
+++ b/lib/components.rb
@@ -127,24 +127,50 @@ class ConnectionComponent < Component
   field :buf, default: ''   # String of pending output
 end
 
-# Used to specify a script to run when an entity is moved into a
-# ContainerComponent.  Think walking into a room, or putting an item in a bag.
-# Technically it could even be when a mob picks up an item.
-class OnEnterComponent < Component
-  # due to compositing architecture, we're going to permit multiple triggers
-  not_unique
-
-  # This points at the entity id for the script that should be run when this
-  # entity is entered.
-  field :script
-end
-
-class OnExitComponent < Component
-  # due to compositing architecture, we're going to permit multiple triggers
+# Used to run scripts when specific events occur in the world.
+#
+class HookComponent < Component
+  # due to compositing architecture, we're going to permit multiple hooks
   not_unique
 
-  # This points at the entity id for the script that should be run when this
-  # entity is exited.
+  # Event when this script should be run
+  #
+  # will_enter: Entity (entity) will be added to (dest) entity's
+  #             ContainerComponent.  May be blocked by returning :deny.
+  #             This hook will be called before the character performs 'look'
+  #             in the dest room.
+  #
+  #             Script arguments:
+  #               entity: Entity being moved
+  #               src:    entity's current location; may be nil
+  #               dest:   destination entity
+  #
+  # on_enter:   Entity (entity) has been added to (here) entity's
+  #             ContainerComponent.
+  #             This hook will be caled after the character performs 'look' in
+  #             here.
+  #
+  #             Script arguments:
+  #               entity: Entity that was added
+  #               here:   entity's current location
+  #
+  # will_exit:  Entity (entity) will removed from (src) entity's
+  #             ContainerComponent.  May be blocked by returning :deny.
+  #             Script arguments:
+  #
+  #               entity: Entity being moved
+  #               src:    entity's current location; may be nil
+  #               dest:   destination entity
+  #
+  # on_exit:    Entity (entity) has been removed from (here) entity's
+  #             ContainerComponent.
+  #
+  #             Script arguments:
+  #               entity: Entity that was removed
+  #               here:   entity's current location
+  field :event, valid: %i{ will_enter on_enter will_exit on_exit }
+
+  # This points at the entity id for the script that should be run
   field :script
 end
 
@@ -163,7 +189,7 @@ class TeleporterComponent < Component
 # It is added by the teleporter script
 class TeleportComponent < Component
   # destination entity
-  field :dest, valid: proc { |v| World.entity_exists?(v) }
+  field :dest, valid: proc { |v| v.nil? or World.entity_exists?(v) }
 
   # Time at which they should be teleported
   field :at
diff --git a/lib/script.rb b/lib/script.rb
index 6b79dba..3119430 100644
--- a/lib/script.rb
+++ b/lib/script.rb
@@ -77,26 +77,67 @@ class Script
   InstanceMethodWhitelist = %i{
     ! % & | ^ + - * / == =~ === < > <= >= << >> [] []=
     first each map inject nil?
+    __id__
   }
 
-  # UnsafeScript
+  # ProhibitedScriptElementError
   #
-  # Exception we raise for untrusted elements in the script
-  class UnsafeScript < RuntimeError
-    attr_reader :root, :node
+  # Parent class for exceptions we raise when a script is determined to be
+  # unsafe.  This is mostly helper functions for the specific exceptions we
+  # encounter.
+  class ProhibitedScriptElementError < ArgumentError
+    def initialize(error, error_node)
+      @error = error
+      @error_node = error_node
+
+      line = error_node.loc.line
+      column = error_node.loc.column + 1
+      msg = "At line #{line}, column #{column}, #{error}\n"
+      msg << clang_error
 
-    def initialize(msg, root, node)
-      @root = root
-      @node = node
       super(msg)
     end
+
+    # clang_error
+    #
+    # Generate a clang formatted error for this exception
+    def clang_error
+      loc = @error_node.loc
+      expr = loc.expression
+      line = loc.line
+      column = loc.column + 1
+      prefix = "#{expr.source_buffer.name}:#{line}:"
+      out = "#{prefix}#{column}: error: #{@error}\n"
+      out << "#{prefix} #{expr.source_line}\n"
+      out << "#{prefix} "
+      out << ' ' * (column - 1)
+      out << "^\n"
+    end
+  end
+  class ProhibitedMethod < ProhibitedScriptElementError;
+    def initialize(method, error_node)
+      super("prohibitied method call, #{method}.", error_node)
+    end
+  end
+  class ProhibitedNodeType < ProhibitedScriptElementError;
+    def initialize(type, error_node)
+      super("prohibitied ruby code, #{type}.", error_node)
+    end
   end
 
   # initialize
   #
   # Create a new script with the source provided
-  def initialize(script)
-    @source = script.clone.freeze
+  #
+  # Arguments:
+  #   source: script source
+  #
+  # Parameters:
+  #   freeze: set to false to not freeze the class; ONLY FOR TESTING
+  #
+  def initialize(source, freeze: true)
+    @source = source.clone.freeze
+    safe!(freeze: freeze)
   end
 
   # Make yaml create a Script instance for `!script` tags
@@ -107,6 +148,7 @@ def init_with(coder)
     raise RuntimeError, "invalid script: %s " %
         [ coder.send(coder.type).inspect ] unless coder.type == :scalar
     @source = coder.scalar.to_s.freeze
+    @safe = false   # trust nothing
     safe!
   end
 
@@ -115,34 +157,59 @@ def encode_with(coder)
     coder.scalar = @source
   end
 
-  # safe!
-  #
-  # Check to see if the script source is safe to be run
-  def safe!
-    ast = Parser::CurrentRuby.parse(@source)
-    node_safe!(ast)
-    true
-  end
-
   # call
   #
   # Call the script with provided arguments.
-  def call(entity: nil, actor: nil)
-    # XXX for the moment, we'll verify each time.  Later let's clean this up
-    # and only verify safe once, cache the result, and freeze the class.
-    safe!
+  def call(args={})
+    raise "refusing to run unsafe script: #{self}" unless @safe
 
     # eval the code within the Sandbox, but first set the entity & actor
     # provided.
     Sandbox.instance_exec(@source) do |source|
-      entity = entity
-      actor = actor
+      args = args
       eval(source)
     end
   end
 
   private
 
+  # safe!
+  #
+  # Check to see if the script source is safe to be run
+  #
+  # Parameters:
+  #   freeze: should the instance be frozen; ONLY FOR TESTING
+  #
+  def safe!(freeze: true)
+    begin
+      ast = parse(@source)
+      node_safe!(ast)
+      @safe = true
+    ensure
+      # After verifying the script is safe, we freeze the instance so that it
+      # cannot easily be modified later to run unsafe code.
+      self.freeze if freeze
+    end
+  end
+
+  # parse
+  #
+  # Wrapper around Parser::Current#parse() to silence the stderr output on
+  # syntax errors.  Opened https://github.com/whitequark/parser/issues/644
+  # but they didn't feel it was necessary to add that option to the top-level.
+  #
+  # Arguments: None
+  #
+  # Returns: Parser::AST::Node
+  def parse(source)
+    parser = Parser::CurrentRuby.new
+    parser.diagnostics.all_errors_are_fatal = true
+    parser.diagnostics.ignore_warnings      = true
+    source_buffer = Parser::Source::Buffer.new('script')
+    source_buffer.source = source
+    parser.parse(source_buffer)
+  end
+
   # node_safe!
   #
   # Verify that a Parser::AST::Node is in our whitelist.  If it is not, raise
@@ -158,15 +225,12 @@ def node_safe!(node, lvars: {})
       # methods from the sandbox.
       receiver, method = node.children
       whitelist = receiver ? InstanceMethodWhitelist : Sandbox::ScriptMethods
-      raise UnsafeScript
-          .new("method not permitted: #{method} in #{node}", @ast, node) unless
-              whitelist.include?(method)
-
+      raise ProhibitedMethod.new(method, node) unless
+          whitelist.include?(method)
     else
 
       # If the node type is not supported, raise an exception.
-      raise UnsafeScript
-          .new("node type not in whitelist: #{node.type}", @ast, node) unless
+      raise ProhibitedNodeType.new(node.type, node) unless
           NodeTypeWhitelist.include?(node.type)
     end
     node.children.each { |c| node_safe!(c) }
diff --git a/lib/script/sandbox.rb b/lib/script/sandbox.rb
index 666d72a..9fb6a10 100644
--- a/lib/script/sandbox.rb
+++ b/lib/script/sandbox.rb
@@ -22,7 +22,7 @@ module Script::Sandbox
   # First things in this list are actually variables we get when the script is
   # run.  We act like they're methods so that Script#safe! doesn't think
   # they're unauthorized methods.
-  ScriptMethods = [ :entity, :actor ]
+  ScriptMethods = [ :args ]
 
   # Pull in the standard logging functions
   extend Helpers::Logging
diff --git a/lib/system/connections.rb b/lib/system/connections.rb
index 826dd91..2371145 100644
--- a/lib/system/connections.rb
+++ b/lib/system/connections.rb
@@ -5,10 +5,6 @@ module System::Connections
   IDLE_TIMEOUT = 5 * 60           # XXX not implemented
   DISCONNECT_TIMEOUT = 30 * 60
 
-  def self.view
-    @view ||= World.get_view(all: ConnectionComponent)
-  end
-
   def self.update(entity, conn_comp)
     conn = conn_comp.conn or return
 
diff --git a/lib/world/helpers.rb b/lib/world/helpers.rb
index 95dfbd8..146013d 100644
--- a/lib/world/helpers.rb
+++ b/lib/world/helpers.rb
@@ -92,29 +92,31 @@ def load_entities(path, area: nil)
     loader.load(path: path, area: area)
     loader.finish
   end
-
-  # call_on_enter_scripts
   #
   # Call any on_enter scripts defined on location
-  def call_on_enter_scripts(location: nil, actor: nil)
+  def call_on_exit_scripts(location: nil, actor: nil)
     fault 'no actor' unless actor
     fault 'no location' unless location
 
-    get_components(location, :on_enter).each do |trig|
+    get_components(location, :on_exit).each do |trig|
       call_script(trig.script, entity: location, actor: actor)
     end
   end
 
-  # call_on_exit_scripts
+  # fire_hooks
   #
-  # Call any on_enter scripts defined on location
-  def call_on_exit_scripts(location: nil, actor: nil)
-    fault 'no actor' unless actor
-    fault 'no location' unless location
-
-    get_components(location, :on_exit).each do |trig|
-      call_script(trig.script, entity: location, actor: actor)
+  # Call any scripts associated with the hook event on this entity
+  #
+  # Returns:
+  #   true: one of the scripts returned :deny
+  #   false: no script returned :deny
+  def fire_hooks(entity, event, args={})
+    get_components(entity, :hook).each do |hook|
+      next unless hook.event == event
+      result = call_script(hook.script, args)
+      return true if result == :deny
     end
+    false   # not denied
   end
 
   # call_script
diff --git a/lib/world/script_safe_helpers.rb b/lib/world/script_safe_helpers.rb
index 56445aa..2e45cb9 100644
--- a/lib/world/script_safe_helpers.rb
+++ b/lib/world/script_safe_helpers.rb
@@ -34,22 +34,52 @@ def exit_name(passage)
     (keywords & World::CARDINAL_DIRECTIONS).first or keywords.first
   end
 
-  # place one entity inside another entity's contents
-  def move_entity(dest: nil, entity: nil)
+  # move_entity
+  #
+  # Move an entity into the ContainerComponent of another entity.  If the
+  # entity being moved already resides within another entity's
+  # ContainerComponent, first remove it from it's existing container.
+  #
+  # This method will also fire the following hooks in order:
+  #   * will_exit;  if script returns :deny, will not move
+  #   * will_enter; if script returns :deny, will not move
+  #   * <move the entity>
+  #   * on_exit
+  #   * on_enter
+  #
+  def move_entity(dest: nil, entity: nil, look: false)
     container = get_component!(dest, :container)
     location = get_component!(entity, :location)
-
-    if old = location.entity and src = get_component(old, :container)
-      debug "moving #{entity} from #{old} to #{dest}"
-      World.call_on_exit_scripts(actor: entity, location: old)
-      src.contents.delete(entity)
-    else
-      debug "moving #{entity} to #{dest}"
+    src = location.entity
+
+    # run the will_exit/will_enter hooks, and check if the movement is denied
+    # by any of them.
+    if src
+      denied = World.fire_hooks(src, :will_exit,
+          entity: entity, src: src, dest: dest)
+      return if denied
     end
 
+    # destination room can also deny entry
+    denied = World.fire_hooks(dest, :will_enter,
+        entity: entity, src: src, dest: dest)
+    return if denied
+
+    # move the entity
+    get_component(src, :container)&.contents&.delete(entity) if src
     location.entity = dest
-    container.contents << entity
-    World.call_on_enter_scripts(actor: entity, location: dest)
+    get_component!(dest, :container).contents << entity
+
+    # notify the src location that the entity has left
+    World.fire_hooks(src, :on_exit, entity: entity, here: src) if src
+
+    # perform the look for the entity if it was requested
+    # XXX kludge for right now
+    send_to_char(char: entity, buf: Command.run(entity, 'look')) if look
+
+    # notify the room of the arrival
+    World.fire_hooks(dest, :on_enter, entity: entity, here: dest)
+    nil
   end
 
   # get a/all entities from ++pool++ that have keywords that match our
diff --git a/spec/lib/script_spec.rb b/spec/lib/script_spec.rb
index 718166e..8e7a3ef 100644
--- a/spec/lib/script_spec.rb
+++ b/spec/lib/script_spec.rb
@@ -1,17 +1,48 @@
 describe Script do
-  describe '#safe!' do
+  # various scripts
+  #
+  # This is a sampling of all the elements we support/reject in scripts.  This
+  # shared example is used for #initialize, and yaml load to verify that the
+  # scripts are safely handled under each condition.
+  #
+  shared_examples 'various scripts' do
+
+    # script is safe to run
     shared_examples 'no error' do
       it 'will not raise an error' do
-        expect { script.safe! }.to_not raise_error
+        expect { script }.to_not raise_error
+      end
+      it 'will freeze the instance' do
+        expect(script).to be_frozen
+      end
+    end
+
+    # syntax error in the ruby syntax
+    shared_examples 'syntax error' do
+      it 'will raise Parser::SyntaxError' do
+        expect { script }
+            .to raise_error(Parser::SyntaxError)
       end
     end
 
-    shared_examples 'raise error' do
-      it 'will raise an UnsafeScript error' do
-        expect { script.safe! }.to raise_error(Script::UnsafeScript)
+    # calling a prohibitied method
+    shared_examples 'method error' do
+      it 'will raise a ProhibitedMethod error' do
+        expect { script }
+            .to raise_error(Script::ProhibitedMethod)
       end
     end
 
+    # using an unsupported ruby feature (const, backticks)
+    shared_examples 'node error' do
+      it 'will raise a ProhibitedNodeType error' do
+        expect { script }
+            .to raise_error(Script::ProhibitedNodeType)
+      end
+    end
+
+    # All of the test cases, broken down into description, code, and the
+    # expected result.
     [
         # base types
         { desc: 'nil', code: 'nil', expect: 'no error' },
@@ -32,14 +63,14 @@
 
         # constants are not permitted; makes everything much easier to
         # implement scripting safely.
-        { desc: 'constant World', code: 'World', expect: 'raise error' },
+        { desc: 'constant World', code: 'World', expect: 'node error' },
         { desc: 'constant World::Helpers',
           code: 'World::Helpers',
-          expect: 'raise error' },
-        { desc: 'constant File', code: 'File', expect: 'raise error' },
-        { desc: 'constant Kernel', code: 'Kernel', expect: 'raise error' },
-        { desc: 'constant Object', code: 'Object', expect: 'raise error' },
-        { desc: 'constant Script', code: 'Script', expect: 'raise error' },
+          expect: 'node error' },
+        { desc: 'constant File', code: 'File', expect: 'node error' },
+        { desc: 'constant Kernel', code: 'Kernel', expect: 'node error' },
+        { desc: 'constant Object', code: 'Object', expect: 'node error' },
+        { desc: 'constant Script', code: 'Script', expect: 'node error' },
 
         # assignment
         { desc: 'assign local var',
@@ -47,16 +78,16 @@
           expect: 'no error' },
         { desc: 'assign instance var',
           code: '@instance_var = true',
-          expect: 'raise error' },
+          expect: 'node error' },
         { desc: 'assign class var',
           code: '@@class_var = true',
-          expect: 'raise error' },
+          expect: 'node error' },
         { desc: 'assign constant',
           code: 'Constant = true',
-          expect: 'raise error' },
+          expect: 'node error' },
         { desc: 'assign global variable',
           code: '$global_var = true',
-          expect: 'raise error' },
+          expect: 'node error' },
 
         { desc: 'multiple assignment to local variables',
           code: 'a,b = [1,2]',
@@ -92,12 +123,12 @@
         # module & class creation/modification
         { desc: 'module creation/modification',
           code: 'module Meep; end',
-          expect: 'raise error' },
+          expect: 'node error' },
         { desc: 'class creation/modification',
           code: 'class Meep; end',
-          expect: 'raise error' },
+          expect: 'node error' },
         { desc: 'singleton class',
-          expect: 'raise error',
+          expect: 'node error',
           code: <<~CODE
             class << self
               def breakout
@@ -109,18 +140,18 @@ def breakout
 
         # method (un)definition
         { desc: 'define instance method',
-          expect: 'raise error',
+          expect: 'node error',
           code: 'def xxx; end' },
         { desc: 'undefine instance method',
-          expect: 'raise error',
+          expect: 'node error',
           code: 'undef :xxx' },
         { desc: 'define singleton method',
-          expect: 'raise error',
+          expect: 'node error',
           code: 'def self.xxx; end' },
 
         # aliasing
         { desc: 'method aliasing',
-          expect: 'raise error',
+          expect: 'node error',
           code: 'alias xxx yyy' },
 
         ## flow control
@@ -211,62 +242,94 @@ def breakout
           code: '[].map { |v| v + 1 }'},
 
         { desc: 'send non-whitelist command to receiver',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'a.eval("File")' },
         { desc: 'sending non-whitelist command with no receiver',
           code: 'eval("File")',
-          expect: 'raise error' },
+          expect: 'method error' },
 
         # regression prevention
         { desc: 'Symbol#to_proc',
-          expect: 'raise error',
+          expect: 'method error',
           code: ':send.to_proc.call(:eval, :puts, "hello world")' },
         { desc: 'back-tick shell command',
-          expect: 'raise error',
+          expect: 'node error',
           code: '`ls`' },
         { desc: '%x{} shell command',
-          expect: 'raise error',
+          expect: 'node error',
           code: '%x{ls}' },
         { desc: 'system() command',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'system("ls")' },
         { desc: 'exec() command',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'exec("ls")' },
         { desc: 'File.open()',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'File.open("/etc/passwd")' },
         { desc: 'eval',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'eval("File")' },
         { desc: 'instance_eval',
-          expect: 'raise error',
+          expect: 'method error',
           code: 'instance_eval("File")' },
-        { desc: 'send()',
-          expect: 'raise error',
+        { desc: 'send()', expect: 'method error',
           code: 'send(:eval, "File")' },
-        { desc: 'require()',
-          expect: 'raise error',
-          code: 'require("pry")' },
+        { desc: 'require()', expect: 'method error', code: 'require("pry")' },
+        { desc: 'open()', expect: 'method error', code: 'open("/etc/passwd")' },
 
         # teleporter examples
         { desc: 'teleporter enter',
           expect: 'no error',
-          code: <<~CODE
-            porter = get_component(entity, :teleporter) or return
-            port = get_component!(actor, :teleport)
+          code: <<~'CODE'
+            here = args[:here]
+            unless porter = get_component(here, :teleporter)
+              error "#{here} missing teleporter component"
+              return
+            end
+            entity = args[:entity]
+            port = get_component!(entity, :teleport)
             port[:dest] = porter[:dest]
             port[:at] = now + porter[:delay]
           CODE
         },
         { desc: 'teleporter exit',
           expect: 'no error',
-          code: 'remove_component(actor, :teleport)' },
+          code: 'remove_component(args[:entity], :teleport)' },
+
+        # syntax error
+        { desc: 'syntax error', code: '1 = 2', expect: 'syntax error' },
+
     ].each do |desc: nil, code: nil, expect: nil|
       context(desc) do
-        let(:script) { Script.new(code) }
+        let(:source) { code }
         include_examples(expect)
       end
     end
   end
+
+  describe '#initialize' do
+    let(:script) { Script.new(source) }
+    include_examples 'various scripts'
+  end
+
+  describe 'loaded from yaml' do
+    let(:script) do
+      if source
+        YAML.load("!script |\n#{source.indent(2)}")
+      else
+        YAML.load("!script")
+      end
+    end
+    include_examples 'various scripts'
+  end
+
+  describe '#call(args={})' do
+    it 'will execute the script & pass the arguments' do
+      script = Script.new('args[0] = :passed')
+      arg = [:failed]
+      script.call(arg)
+      expect(arg).to eq([:passed])
+    end
+  end
 end
diff --git a/spec/lib/world/helpers_spec.rb b/spec/lib/world/helpers_spec.rb
index be57ef6..c56b764 100644
--- a/spec/lib/world/helpers_spec.rb
+++ b/spec/lib/world/helpers_spec.rb
@@ -151,31 +151,111 @@ def entity_snapshot(entity)
   end
 
   context '.move_entity' do
-    let(:script) { Script.new('true') }
+    let(:script) { Script.new('true', freeze: false) }
     let(:script_entity) do
       create_entity(components: ScriptComponent.new(script: script))
     end
     let(:dest) { create_entity(base: 'base:room') }
+    let(:src) { create_entity(base: 'base:room') }
+    before(:each) { move_entity(entity: leo, dest: src) }
 
-    context 'when dest has on_enter script' do
-      before(:each) do
-        add_component(dest, OnEnterComponent.new(script: script_entity))
+    def add_hook(entity, event)
+      add_component(entity,
+          HookComponent.new(event: event, script: script_entity))
+    end
+
+    context 'when dest has on_enter hook' do
+      before(:each) { add_hook(dest, :on_enter) }
+
+      it 'will move the entity' do
+        move_entity(entity: leo, dest: dest)
+        expect(entity_location(leo)).to eq(dest)
+      end
+
+      it 'will call on_enter script after moving entity' do
+        expect(script).to receive(:call) do |here: nil, entity: nil|
+          expect(here).to eq(dest)
+          expect(entity).to eq(leo)
+          expect(entity_contents(here)).to include(leo)
+        end
+        move_entity(entity: leo, dest: dest)
+      end
+    end
+
+    context 'when dest has will_enter script' do
+      before(:each) { add_hook(src, :will_exit) }
+
+      it 'will move the entity' do
+        move_entity(entity: leo, dest: dest)
+        expect(entity_location(leo)).to eq(dest)
+      end
+
+      it 'will call will_exit script before moving entity' do
+        expect(script).to receive(:call) do |p|
+          expect(p[:entity]).to eq(leo)
+          expect(p[:src]).to eq(src)
+          expect(p[:dest]).to eq(dest)
+          expect(entity_location(leo)).to eq(src)
+        end
+        move_entity(entity: leo, dest: dest)
       end
 
-      it 'will call on_enter script' do
-        expect(script).to receive(:call).with(entity: dest, actor: leo)
+      context 'script returns :deny' do
+        it 'will not move the entity' do
+          expect(script).to receive(:call)
+              .with(entity: leo, src: src, dest: dest)
+              .and_return(:deny)
+          move_entity(entity: leo, dest: dest)
+          expect(entity_location(leo)).to eq(src)
+        end
+      end
+    end
+
+    context 'when dest has on_exit hook' do
+      before(:each) { add_hook(src, :on_exit) }
+
+      it 'will move the entity' do
+        move_entity(entity: leo, dest: dest)
+        expect(entity_location(leo)).to eq(dest)
+      end
+
+      it 'will call on_exit script after moving entity' do
+        expect(script).to receive(:call) do |here: nil, entity: nil|
+          expect(here).to eq(src)
+          expect(entity).to eq(leo)
+          expect(entity_contents(here)).to_not include(leo)
+        end
         move_entity(entity: leo, dest: dest)
       end
     end
-    context 'when entity location has on_exit script' do
-      before(:each) do
-        add_component(dest, OnExitComponent.new(script: script_entity))
+
+    context 'when entity location has will_exit script' do
+      before(:each) { add_hook(src, :will_exit) }
+
+      it 'will move the entity' do
+        move_entity(entity: leo, dest: dest)
+        expect(entity_location(leo)).to eq(dest)
       end
 
-      it 'will call on_exit script' do
+      it 'will call on_exit script before moving entity' do
+        expect(script).to receive(:call) do |p|
+          expect(p[:entity]).to eq(leo)
+          expect(p[:src]).to eq(src)
+          expect(p[:dest]).to eq(dest)
+          expect(entity_location(leo)).to eq(src)
+        end
         move_entity(entity: leo, dest: dest)
-        expect(script).to receive(:call).with(entity: dest, actor: leo)
-        move_entity(entity: leo, dest: create_entity)
+      end
+
+
+      context 'script returns :deny' do
+        it 'will not move the entity' do
+          expect(script).to receive(:call)
+              .with(entity: leo, src: src, dest: dest)
+              .and_return(:deny)
+          move_entity(entity: leo, dest: dest)
+          expect(entity_location(leo)).to eq(src)
+        end
       end
     end
   end
diff --git a/spec/script/teleporter_spec.rb b/spec/script/teleporter_spec.rb
new file mode 100644
index 0000000..76bd69d
--- /dev/null
+++ b/spec/script/teleporter_spec.rb
@@ -0,0 +1,41 @@
+describe 'teleporter script' do
+  include World::Helpers
+
+  before(:each) { load_test_world }
+  let(:troom) do
+    create_entity(base: [ 'base:room', 'base:act/teleporter' ])
+  end
+  let(:teleporter) { get_component!(troom, :teleporter) }
+  let(:dest) { create_entity(base: 'base:room') }
+  let(:leo) { 'spec:mob/leonidas' }
+  before(:each) { teleporter.dest = dest; teleporter.delay = 60 }
+
+  context 'entity enters teleporter' do
+    before(:each) { move_entity(entity: leo, dest: troom) }
+    it 'will add the teleport component to entity' do
+      expect(get_component(leo, :teleport)).to_not be_nil
+    end
+
+    it 'will set the teleport destination' do
+      expect(get_component(leo, :teleport).dest)
+          .to eq(teleporter.dest)
+    end
+
+    it 'will set the teleport delay' do
+      expect(get_component(leo, :teleport).at.to_f)
+          .to be_within(1).of(Time.now.to_f + teleporter.delay)
+    end
+  end
+
+  context 'entity exits teleporter' do
+    before(:each) do
+      move_entity(entity: leo, dest: troom)
+      expect(get_component(leo, :teleport)).to_not be_nil
+      move_entity(entity: leo, dest: dest)
+    end
+
+    it 'will remove the teleport component to entity' do
+      expect(get_component(leo, :teleport)).to eq(nil)
+    end
+  end
+end

From 6cba3ed49b4d0996566e02abc05fdc9ab3743d4d Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 30 Dec 2019 20:53:13 -0600
Subject: [PATCH 5/6] restoring System::Connections.view

---
 lib/system/connections.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/system/connections.rb b/lib/system/connections.rb
index 2371145..826dd91 100644
--- a/lib/system/connections.rb
+++ b/lib/system/connections.rb
@@ -5,6 +5,10 @@ module System::Connections
   IDLE_TIMEOUT = 5 * 60           # XXX not implemented
   DISCONNECT_TIMEOUT = 30 * 60
 
+  def self.view
+    @view ||= World.get_view(all: ConnectionComponent)
+  end
+
   def self.update(entity, conn_comp)
     conn = conn_comp.conn or return
 

From 4c0f0a99109f2b455cb2c6302422e529df44d7af Mon Sep 17 00:00:00 2001
From: "David M. Lary" <dmlary@gmail.com>
Date: Mon, 30 Dec 2019 21:39:07 -0600
Subject: [PATCH 6/6] Teleport system

---
 data/world/base.yml              |  2 +-
 lib/components.rb                |  2 +-
 lib/system.rb                    |  1 +
 lib/system/base.rb               |  2 +-
 lib/system/teleport.rb           | 21 ++++++++++++
 lib/world.rb                     |  1 +
 spec/lib/system/teleport_spec.rb | 59 ++++++++++++++++++++++++++++++++
 spec/script/teleporter_spec.rb   |  2 +-
 8 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 lib/system/teleport.rb
 create mode 100644 spec/lib/system/teleport_spec.rb

diff --git a/data/world/base.yml b/data/world/base.yml
index f3bffb5..413cf1a 100644
--- a/data/world/base.yml
+++ b/data/world/base.yml
@@ -135,7 +135,7 @@
       entity = args[:entity]
       port = get_component!(entity, :teleport)
       port[:dest] = porter[:dest]
-      port[:at] = now + porter[:delay]
+      port[:time] = now + porter[:delay]
 
 - id: base:script/teleporter/exit
   components:
diff --git a/lib/components.rb b/lib/components.rb
index 17395fb..719ebfc 100644
--- a/lib/components.rb
+++ b/lib/components.rb
@@ -192,7 +192,7 @@ class TeleportComponent < Component
   field :dest, valid: proc { |v| v.nil? or World.entity_exists?(v) }
 
   # Time at which they should be teleported
-  field :at
+  field :time
 end
 
 # Component that holds a script
diff --git a/lib/system.rb b/lib/system.rb
index ecd1635..4b2c635 100644
--- a/lib/system.rb
+++ b/lib/system.rb
@@ -5,3 +5,4 @@ module System
 require_relative 'system/command_queue'
 require_relative 'system/connections'
 require_relative 'system/spawner'
+require_relative 'system/teleport'
diff --git a/lib/system/base.rb b/lib/system/base.rb
index b4e367e..faccf1d 100644
--- a/lib/system/base.rb
+++ b/lib/system/base.rb
@@ -2,5 +2,5 @@
 
 module System::Base
   include ::Helpers::Logging
-  extend ::World::Helpers
+  include ::World::Helpers
 end
diff --git a/lib/system/teleport.rb b/lib/system/teleport.rb
new file mode 100644
index 0000000..b95d380
--- /dev/null
+++ b/lib/system/teleport.rb
@@ -0,0 +1,21 @@
+module System::Teleport
+  extend System::Base
+
+  class << self
+    def view
+      @view ||= World.get_view(all: :teleport)
+    end
+
+    def update(entity, teleport)
+      if !teleport.dest or !teleport.time
+        error("invalid teleport (#{teleport.to_h}) on #{entity}; removing")
+        remove_component(entity, teleport)
+        return
+      end
+
+      return if teleport.time > Time.now
+
+      move_entity(entity: entity, dest: teleport.dest, look: true)
+    end
+  end
+end
diff --git a/lib/world.rb b/lib/world.rb
index 0baad2f..4cc8a48 100644
--- a/lib/world.rb
+++ b/lib/world.rb
@@ -123,6 +123,7 @@ def register_systems
       @systems << System::Connections
       @systems << System::CommandQueue
       @systems << System::Spawner
+      @systems << System::Teleport
     end
 
     # update
diff --git a/spec/lib/system/teleport_spec.rb b/spec/lib/system/teleport_spec.rb
new file mode 100644
index 0000000..1a3a643
--- /dev/null
+++ b/spec/lib/system/teleport_spec.rb
@@ -0,0 +1,59 @@
+describe System::Teleport do
+  include World::Helpers
+
+  before(:each) { load_test_world }
+  let(:leo) { 'spec:mob/leonidas' }
+  let(:teleport) { get_component!(leo, :teleport) }
+  let(:dest) { create_entity(base: 'base:room') }
+  let(:src) { create_entity(base: 'base:room') }
+  before(:each) { move_entity(entity: leo, dest: src) }
+
+  def run_update
+    System::Teleport.update(leo, teleport)
+  end
+
+  shared_examples 'error' do
+    before(:each) { run_update }
+    it 'will remove the teleport component' do
+      expect(get_component(leo, :teleport)).to eq(nil)
+    end
+    it 'will not move the entity' do
+      expect(entity_location(leo)).to eq(src)
+    end
+  end
+
+  context 'with no time set' do
+    before(:each) { teleport.dest = dest }
+    include_examples 'error'
+  end
+
+  context 'with no dest set' do
+    before(:each) { teleport.time = Time.now }
+    include_examples 'error'
+  end
+
+  context 'when time is in the future' do
+    before(:each) do
+      teleport.dest = dest
+      teleport.time = Time.now + 1
+      run_update
+    end
+
+    it 'will not move the entity' do
+      expect(entity_location(leo)).to eq(src)
+    end
+  end
+
+  context 'when time is in the past' do
+    before(:each) do
+      teleport.dest = dest
+      teleport.time = Time.now - 1
+      run_update
+    end
+
+    it 'will move the entity' do
+      expect(entity_location(leo)).to eq(dest)
+    end
+  end
+end
+
diff --git a/spec/script/teleporter_spec.rb b/spec/script/teleporter_spec.rb
index 76bd69d..51c2e3b 100644
--- a/spec/script/teleporter_spec.rb
+++ b/spec/script/teleporter_spec.rb
@@ -22,7 +22,7 @@
     end
 
     it 'will set the teleport delay' do
-      expect(get_component(leo, :teleport).at.to_f)
+      expect(get_component(leo, :teleport).time.to_f)
           .to be_within(1).of(Time.now.to_f + teleporter.delay)
     end
   end