Instructions print via game

[drewish]

This is a bit of a work in progress. I started out trying to get the Instruction objects sending their output to the game so my subclass could capture it. At that point it seemed pretty natural to try to handle a bit of #21. I didn't go all the way but it removes the need for withIO for output.

The commits should be pretty self contained so probably best to go through that each of those. If you like the direction I can keep going and try to get the input converted to an optional option to the Game constructor.

[drewish]

Remind me, what is the difference in Ruby between making a method public (as in your commit) and making it private but the declaring it public after all (as in my original)?

I think the end result is the same but just declaring it public to start results in less surprises. I remember reading through the code the first time and thinking "how is this working if these a private?" before coming across the access change later. Since they ended up public it seemed better to make the interface clearer from the start.

[MikeTaylor]

Cool, thanks for clarifying that. I agree, the way you have it is better.

[drewish]

I'm not sure if the other comments answered this or not:

Overall, the approach for compilation looks good to me. I think that play should follow the same model, though, and use explicit input and output parameters, rather than relying on a new :output option. We want the compilation and playing APIs to be compatible, right?

I'm not sure I understand this. Play should be using the :read_file and :output options. Or are you saying that the compilation should stop sending stuff to $stdout and use the :output option instead?

[MikeTaylor]

Sorry, this has got complicated because of all the different comment-threads!

All I am saying is that if the compilation API is now compile_to(stream), then surely the playing API should be play_to(stream) as well, instead of using an output stream hidden in the options block?

But I do see that there is then an asymmetry with input. But remember that :read_file is not just providing an input source: it's a bit more specific than that, it's saying "take the initial set of in-game commands from here, then continue as normal". (Its purpose is to allow you to replay partially-completed solutions, then continue from there manually.) So it's not really the counterpart of your :output.

[drewish]

That last comment makes sense. I hadn't really understood that read_file was just initial commands and then the input was consumed after that. With that in mind I think an :input option would make sense.

[drewish]

There's only a couple of places where the compiler is used, in some tests and then in the scottkit binary. So it would be simple enough to just switch those to the long way

when :play_from_source
  compiled_game = StringIO.new
  compiler = ScottKit::Game::Compiler.new(game, ARGV[0])
  if !compiler.compile_to(compiled_game)
    $stderr.puts "#$0: compilation failed: not playing"
    exitval = 1
  end
  if exitval == 0
    game.load(compiled_game.string)
    play(game)
  end
when :compile
  compiler = ScottKit::Game::Compiler.new(game, ARGV[0])
  if !compiler.compile_to($stdout)
    $stderr.puts "#$0: compilation failed"
    exitval = 1
  end

The thing that seems confusing to me about having game.compile is that it doesn't compile and load the results into the game object. The comments around the compilation methods seem to hint at this:

    # Compiles the specified game-source file, writing the resulting
    # object file to stdout, whence it should be redirected into a
    # file so that it can be played.  Yes, this API is sucky: it would
    # be better if we had a simple compile method that builds the game
    # in memory in a form that can by played, and which can then also
    # be saved as an object file by some other method -- but that
    # would have been more work for little gain.

The Game seems like it wants to be the in memory structure but compiler bypasses it in favor of its own structs.

This PR's grown big enough so I'm not looking to increase the scope any more, just pointing out things have have jumped out at me. If they're things you're interested in changing I'm happy to help.

[drewish]

@MikeTaylor Just wanted to check in with you and see if this is something you're interested in pursuing or not. If not are there commits that you'd accept? I'm happy to just keep it in my fork but would love to land any changes that make sense to you.

[MikeTaylor]

I agree there is a issue here, but it needs thought and I won't be able to give it any for 72 hours. If you can wait, that'd be best for me.

[drewish]

No rush at all. Just wanted to give you an easy way to say no if it wasn’t a direction that makes sense to you.