Add multiple source links for teleporters

[Th30n]

@johnnovak this my initial implementation for the "teleport destinations with multiple sources" feature. Feel free to suggest a different approach or tweaks to the existing one. Otherwise, when/if the mechanics are good to go, I'll update the manual, changelog, etc.

[johnnovak]

This is absolutely outstanding work, @Th30n! Sorry for the delayed response, but my 95 year old father had died and I need to support my mother during this period and organise a lot of things... I'll be back in business around 20 Oct, then I'll review and test this properly.

[Th30n]

This is absolutely outstanding work, @Th30n! Sorry for the delayed response, but my 95 year old father had died and I need to support my mother during this period and organise a lot of things... I'll be back in business around 20 Oct, then I'll review and test this properly.

I am sorry to hear that! That is some nice age under your father's belt, I hope he had a good life. My sincere condolences to you and your family.

[johnnovak]

This is absolutely outstanding work, @Th30n! Sorry for the delayed response, but my 95 year old father had died and I need to support my mother during this period and organise a lot of things... I'll be back in business around 20 Oct, then I'll review and test this properly.

I am sorry to hear that! That is some nice age under your father's belt, I hope he had a good life. My sincere condolences to you and your family.

Thanks @Th30n, I appreciate it. He had a pretty good run, all in all.

I went through the PR today on my phone, and I couldn't have done it better myself. I might request some minor changes later, but this is how I thought I'd implement it at the high level.

So feel free to proceed with the doco updates in the meantime. If you have time you could do some regression testing, e.g. make sure undo/redo works as expected when creating/deleting/overwriting links by various operations (move, cut/copy/paste, nudge, resize level, delete level, etc.) In theory it should be fine, but proper link handling can be finicky from experience, so best to test it extensively

I think we should allow multiple sources for all link types.

I'm also thinking it could be handy to show some extra visual marker in multi-source destination cells, but I'm not sure how... I'd need to experiment with this a bit, and perhaps it's outside of the scope of the current PR anyway.

[Th30n]

Thanks for the review, I'll work on this over the weekend

[johnnovak]

Thanks for the review, I'll work on this over the weekend

Sounds great @Th30n, no rush. May I suggest to address issues in separate commits (as opposed to force pushing) to make the review easier? Also, it's best to make every single commit compilable for easy bisecting (if needed).

I've upgraded to Nim 1.6.8, I think you should do the same (there will be some naming related warnings; just ignore those for now).

[Th30n]

Will do that.

Btw, I only force push after rebasing (on master in this case), and I only rebase to keep the commit history linear so as to make git log and git bisect even easier to work with.

[Th30n]

I think we should allow multiple sources for all link types.

I've been thinking about this, and now I'm not so sure about it. I think it makes sense for one way mechanisms, like teleporters typically are. I guess it would then also make sense for pits since you typically enter on one end cannot return at the exit point. But what about stairs and doors? They are typically bidirectional, and games usually don't allow you to select the final destination i.e. they utilize single source & single destination in bidirectional links.

The only example I can think of that breaks this simplicity is Might & Magic via its teleportation mechanism in towns. Each town has an area with a teleportation transport service where the player can pick (and pay) to be teleported to another town of choice.

In summary, I think we should postpone the decision to allow other link types having multiple sources for now.

I'm also thinking it could be handy to show some extra visual marker in multi-source destination cells, but I'm not sure how... I'd need to experiment with this a bit, and perhaps it's outside of the scope of the current PR anyway.

I agree, I think it would also be great to somehow highlight all the sources on the current floor when hovering over a destination.

[johnnovak]

Hey @Th30n, thanks for addressing my comments, this is coming together very nicely! 🎉 I noticed a few very minor remaining naming inconsistencies, plus two issues that affect functionality after doing some further testing.

Thanks for having a stab at updating the documentation too! I realised I haven't mentioned it anywhere that the doco is generated with Sphinx and lives in the https://github.com/johnnovak/gridmonger-site repo. I'll document that to make it clear, but don't worry about it, I'll update the doco based on your input after we merged this in. I have your current changes safely stashed away on a local branch, please feel free to remove the doco updates from the commit history in this repo.

Then I'll do the release, plus I'll automate it a bit more because right now it's a very manual and error prone process. But... don't worry about that either for now, I'll take care of it 😄

In summary, I think we should postpone the decision to allow other link types having multiple sources for now.

I agree with your analysis, it's best to postpone it, after all. 👍🏻

I think it would also be great to somehow highlight all the sources on the current floor when hovering over a destination.

Yep, I was thinking of something along these lines. This could be an enhancement further down the road and it requires experimentation, so let's not overload the current PR with this stuff. I suggest to focus on getting this first iteration of the feature in first.

So the two issues that need addressing:

Issue 1 — Jumping back and forth with G for the first time (edge case)

1. Create a teleport source
2. Link it to a teleport destination
3. Create a second teleport source, and link it to the same destination
4. Place the cursor at the second teleport source, and press G — the cursor jumps to the destination, this is OK
5. Now press G again — the cursor jumps to the first teleport source, which is wrong; it should have jumped to the second (the last location)

After this, when selecting between the teleport sources and pressing G repeatedly, the last source position is correctly remembered. The issue only happens when using the G repeatedly on a newly created multi-source set of linked teleports, and using any other source than the first as the very first starting point.

Issue 2 — Trail mode compatibility

When trail mode (T key) is on, and G is pressed on a destination cell that has multiple sources, the trail drawing should be temporarily disabled while in teleport source selection mode. When Enter or Esc is pressed, trail mode should be re-enabled (and therefore the trail will be drawn in the selected cell only after exiting source select mode).

[johnnovak]

Thanks for adding this great feature @Th30n and addressing all the comments! Merged. I'll update the doco in the next few days and release a new version.

[Th30n]

Awesome, thanks!