[1812] multiple pre-alpha updates #11540
Conversation
philcampeau
added
new games
| module Game | ||
| module G1812 | ||
| module Step | ||
| class BuyTrain < Engine::Step::BuyTrain |
There was a problem hiding this comment.
Could this class inherit from G1867::Step::BuyTrain? You've got several methods that are cut'n'pasted from that class.
There was a problem hiding this comment.
100% correct. I wasn't sure how to inherit a couple of methods directly from Engine::Step::BuyTrain, but I think I got it now. I needed to inherit from Engine because the 1867 methods included code I didn't want, which prevented me from calling super. Let me know if I can improve this, it's in update fixes buy_train.rb inheritance
|
|
||
| owed = @game.pay_interest!(entity) | ||
| if owed | ||
| @game.unpaid_loan(entity, owed) |
There was a problem hiding this comment.
Refactoring G1867::Step::LoanOperations to extract a (private) unpaid_interest method called at this point would mean that only that method would need to be redone in this class, instead of copying all of the code in this method.
lib/engine/game/g_1812/game.rb
Outdated
| revenue += 10 if corp.assigned?(p4_company) && stops.find { |s| C18_HEX.include?(s.hex.id) } | ||
|
|
||
| revenue += 10 if g_train?(train) && corp.assigned?(p3_company) && stops.any? { |s| F3_PORT.include?(s.hex.id) } | ||
| revenue += 10 if g_train?(train) && corp.assigned?(p8_company) && stops.any? { |s| F3_PORT.include?(s.hex.id) } | ||
| revenue += 10 if g_train?(train) && corp.assigned?(p9_company) && stops.any? { |s| H9_PORT.include?(s.hex.id) } | ||
| revenue += 20 if g_train?(train) && corp.assigned?(p12_company) && stops.any? { |s| G6_PORT.include?(s.hex.id) } |
There was a problem hiding this comment.
This very repetitive. Could you make a method that encapsulates these tests?
There was a problem hiding this comment.
Yes, this whole section is still a work in progress, it's not even achieving what I need. Right now my focus is on figuring out how to implement those mine stops on plain track hexes. Once I have those, I'll be able to test and correct this section. I can comment it out for now if that's better.
There was a problem hiding this comment.
Actually, I think I got something. Let me know what you think. Tbh, I feel like it looks more cluttered. The reason I had it as separate lines was they were all clean and easily read/understood.
| { p3_company => F3_PORT, p8_company => F3_PORT, p9_company => H9_PORT, p12_company => G6_PORT }.each do |company, port| | ||
| revenue += (company == p12_company ? 20 : 10) if g_train?(train) && corp.assigned?(company) && stops.any? do |s| | ||
| s.hex.id == port | ||
| end |
There was a problem hiding this comment.
Yeah, this isn't clearer. The HexBonus ability almost matches what you need here (and does exactly for P4), you just need some way of testing that the train is a goods train. Maybe add a trains array to that ability, then in revenue_for test both that you have a matching hex and a matching train.
| include Engine::Step::AutomaticLoan | ||
|
|
||
| def pass! | ||
| Engine::Step::BuyTrain.instance_method(:pass!).bind_call(self) |
There was a problem hiding this comment.
This is a bit clunky. Could you just call super, and override nationalize! in the game class so that it does nothing?
| def buy_train_action(action, _entity = nil) | ||
| @depot_train = action.train.from_depot? | ||
|
|
||
| Engine::Step::BuyTrain.instance_method(:buy_train_action).bind_call(self) | ||
| end |
There was a problem hiding this comment.
You could get rid of this method entirely, by having a post_train_buy method in the game class that does nothing (but with a comment explaining why you've done this).
Before clicking "Create"
masterpinsorarchive_alpha_gameslabel if this change will break existing gamesdocker compose exec rack rubocop -adocker compose exec rack rakeImplementation Notes
Explanation of Change
Screenshots
Any Assumptions / Hacks