Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1862/autorouter] Fix freight trains not being able to auto route #11020

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PistonPercy
Copy link
Contributor

Before clicking "Create"

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

Implementation Notes

A minimal repro is here: https://gist.github.com/PistonPercy/dbbad162540451e8941dfae3e57ccbc9

The error I'm attempting to fix is:

Uncaught []: undefined method `[]' for nil
Caused by: Object { … }
klass runtime.js:589
$Exception_new$1 error.rb:12
$$method_missing basic_object.rb:146
method_missing_stub runtime.js:1563
$$adjust_end game.rb:1618
$$hex_crow_distance game.rb:1631
$$freight_revenue game.rb:1671
$$revenue_for game.rb:1676

which is originating from this line: https://github.com/tobymao/18xx/blob/master/lib/engine/game/g_1862/game.rb#L1618

The issue is the chains from the auto router don't have :hexes, but this code expects there to be. I'm not sure if :hexes is suppose to be there or the other side should be tweaked.

Explanation of Change

This isn't enough to fix 1862 auto routing, but it is the most obvious issue and I figured I'd start small.

A minimal repro is here: https://gist.github.com/PistonPercy/dbbad162540451e8941dfae3e57ccbc9

```
Uncaught []: undefined method `[]' for nil
Caused by: Object { … }
klass runtime.js:589
$Exception_new$1 error.rb:12
$$method_missing basic_object.rb:146
method_missing_stub runtime.js:1563
$$adjust_end game.rb:1618
$$hex_crow_distance game.rb:1631
$$freight_revenue game.rb:1671
$$revenue_for game.rb:1676
```

which is originating from this line: https://github.com/tobymao/18xx/blob/master/lib/engine/game/g_1862/game.rb#L1618

The issue is the chains from the auto router don't have :hexes, but this code
expects there to be. I'm not sure if :hexes is suppose to be there or the other
side should be tweaked.
@roseundy
Copy link
Collaborator

roseundy commented Jul 7, 2024

chain as defined in the auto-router is a completely different thing from chain in the 1862 code.

@PistonPercy
Copy link
Contributor Author

I'm definitely no expert, but it seems to me that the auto-router and 1862 is sharing chains through the Route class and I can see the Route class setting the hexes field on the chains it makes in https://github.com/tobymao/18xx/blob/master/lib/engine/route.rb#L126

If I put a breakpoint on this line https://github.com/tobymao/18xx/blob/master/lib/engine/game/g_1862/game.rb#L1618 and if I manually input a route on my minimal repro game above, I can see:
end_chain = Map(4) { nodes → (2) […], paths → (2) […], hexes → (2) […], id → (2) […] }

If I auto route without this change I can see:
end_chain = Map { nodes → (2) […], paths → (2) […] }

If I auto route with this change, I can see:
end_chain = Map(3) { nodes → (2) […], paths → (2) […], hexes → (2) […] }

If this isn't the right path, what is?

@PistonPercy PistonPercy mentioned this pull request Jul 28, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants