Skip to content

[18esp] fix mz mza cheater token #11721

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

Merged
merged 1 commit into from
May 30, 2025
Merged

Conversation

bentziaxl
Copy link
Collaborator

MZA and MZ share a single slot city in madrid (f24).
When mz is taken over, and MZA was placed as a cheater token, the closing of MZ doesn't remove the extra slot.
added minor code to fix this behaviour.

@bentziaxl bentziaxl requested a review from ollybh May 8, 2025 08:21
@ollybh ollybh added the 18ESP label May 8, 2025
Copy link
Collaborator

@ollybh ollybh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this might not work in all cases.

Would it be possible to do something like this, overriding close_corporation instead of adding the fix_cheater_slot_madrid method?

def close_corporation(corporation, quiet: false)
  if corporation.type == :minor
    corporation.placed_tokens.each do |token|
      city = token.city
      remove_slot = city.tokens.any? { |t| t.cheater? }
      city.delete_token!(token, remove_slot: remove_slot)
    end
  end
  super
end

@bentziaxl
Copy link
Collaborator Author

I think that this might not work in all cases.

Would it be possible to do something like this, overriding close_corporation instead of adding the fix_cheater_slot_madrid method?

def close_corporation(corporation, quiet: false)
  if corporation.type == :minor
    corporation.placed_tokens.each do |token|
      city = token.city
      remove_slot = city.tokens.any? { |t| t.cheater? }
      city.delete_token!(token, remove_slot: remove_slot)
    end
  end
  super
end

there is no cheater? method, the best i could come up with is city.tokens.size > city.normal_slots
as cheater adds an extra token, but not another normal_slots.

@ollybh
Copy link
Collaborator

ollybh commented May 13, 2025

there is no cheater? method, the best i could come up with is city.tokens.size > city.normal_slots as cheater adds an extra token, but not another normal_slots.

My mistake, it should have been token.cheater (without a question mark), not token.cheater?.

@bentziaxl
Copy link
Collaborator Author

bentziaxl commented May 20, 2025

found 2 new minor bugs that i'm adding to this:
1- @game.city_by_id('F24-0-2') breaks when the city has been upgraded to green (so obvious!)
added code to determine the city programatically.

2- the rules say that the first run on 2 permanent needs to connect both aranjuez and madrid, not just aranzuez. fixed the logic to reflect that.

3- change the code to token.cheater as suggested,

add bank info on tenders, fix bug where there one less tender in bank

handle pr comments

fix mz mza cheater token

address pr comments

adress pr, fix more minor bugs

address pr comments

fix error on all 2p trains

fix rubycop
@bentziaxl bentziaxl requested a review from ollybh May 26, 2025 06:55
Copy link
Collaborator

@ollybh ollybh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bentziaxl Once a review has been started, can you avoid force-pushing to the PR branch? It means the reviewer can't see what has been changed in the newer commits.

@bentziaxl bentziaxl merged commit 4b071db into tobymao:master May 30, 2025
1 check passed
@bentziaxl bentziaxl deleted the 18esp_bug_fix branch May 30, 2025 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants