search

zond / godip

A dippy adjudicator in Go.
GNU General Public License v3.0
27 stars 22 forks source link

New Ancient Med (READY FOR PULL) #144

Closed JorenC closed 2 years ago

JorenC commented 3 years ago

I think it should be good.

@tttppp when you have time can you check and let me know?

JorenC commented 3 years ago

Sorry, I added this as a gitignore thought it wouldn't be added to your base - it's just a temp file I made to remind me I still need to do this. I'll remove it!

On Sun, 12 Sep 2021, 10:00 tttppp, @.***> wrote:

@.**** commented on this pull request.

In .gitignore https://github.com/zond/godip/pull/144#discussion_r706788024:

@@ -26,3 +26,4 @@ classical/droidippy/games/.txt test_output_maps generator/.go generator/*map.svg +variants/ancientmediterranean/svg/flags + units.txt

If you want to add a new type of file to this then please make it a bit more generic. e.g. Call the file dev-info.txt or something.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zond/godip/pull/144#pullrequestreview-752121129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXURGJA6YCLEGGURZYATZLUBRMYTANCNFSM5D3HOGDA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

JorenC commented 3 years ago

Or better, please discard that change and leave it. It's an empty file

On Sun, 12 Sep 2021, 10:01 Joren Bredman, @.***> wrote:

Sorry, I added this as a gitignore thought it wouldn't be added to your base - it's just a temp file I made to remind me I still need to do this. I'll remove it!

On Sun, 12 Sep 2021, 10:00 tttppp, @.***> wrote:

@.**** commented on this pull request.

In .gitignore https://github.com/zond/godip/pull/144#discussion_r706788024:

@@ -26,3 +26,4 @@ classical/droidippy/games/.txt test_output_maps generator/.go generator/*map.svg +variants/ancientmediterranean/svg/flags + units.txt

If you want to add a new type of file to this then please make it a bit more generic. e.g. Call the file dev-info.txt or something.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zond/godip/pull/144#pullrequestreview-752121129, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXURGJA6YCLEGGURZYATZLUBRMYTANCNFSM5D3HOGDA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tttppp commented 3 years ago

Or better, please discard that change and leave it. It's an empty file

I'm not sure I understand what you mean by this. Please can you change the file to however you want it to look, and then if I think it's weird I'll let you know.

JorenC commented 3 years ago

Changed it.

tttppp commented 3 years ago

If you ever want to do this without affecting anyone else then create a file at .git/info/exclude with the filename in. It's exactly like the.gitignore file but it's not committed (ie it's just for you).

JorenC commented 2 years ago

@tttppp this should now be fixed as well. I made this as a 'generic' old style ship, I might update it later in the future but for now it's 1000x better than what it was.

This one should now also be good to go?

tttppp commented 2 years ago

This seems to have changes to the hundred map too. Is that deliberate? Is it ready as well?

JorenC commented 2 years ago

There were no real changes, some spaces added to the Hundred.go by my autoformatting, I believe. It's safe to push, but I reverted them anyway.

tttppp commented 2 years ago

Thanks for reverting those changes. The only issue(s) I spotted:

  1. Thebes looks like it has two coasts in the new map (it only has one)
  2. Related - the Nile is really hard to spot.
JorenC commented 2 years ago

@tttppp I've finalized this now,can you pull it?

tttppp commented 2 years ago

Can you check the "Files changed" tab as it looks like there are changes to a couple of other variants in this branch. I don't think it's ready for merging anyway.

tttppp commented 2 years ago

Something's wrong here - there's a one way arrow.

image

I don't think you intended to change anything about the connections, so maybe this issue already existed?

tttppp commented 2 years ago

I've fixed the issue (it was present before your change) and made some minor updates to the unit positions (to avoid overlapping arrows). I lifted the files without using your commit history, because there were a lot of other changes on your master branch. If you want the other (non-ancient med) changes then please can you put them on branches of their own?