Update teleport

[BarDweller]

Ok, so we still don't have suites.. but here's an idea that might tide us over =)

If the source and destination room have the same ownerId, allow teleport between them.

Then rooms could override /go direction into /teleport target-room-id allowing creation of intentional room graphs overlaid over the existing navigable map. Done correctly a room could test if a player is 'playing' or not, so that when players enter from 'outside' the connected-room graph by stumbling into a room on the main graph, they can bounce them to the start room.. For extra credit rooms could remember where a player entered their connected-room graphs, and have them exit back to that location when they quit.. so many options!

---

This change is 

[ilanpillemer]

---

mediator-app/src/main/java/org/gameontext/mediator/MapClient.java, line 176 at r1 (raw file):

        }
    }

return getSite(roomId) == null ? null : site.getOwner();
would make easier on the eye?

[ilanpillemer]

So a room can delegate go commands to teleport commands, effectively keeping a person only in rooms (wherever they are) controlled by the room. Can the owner control what the doors look like though?

[pavittr]

I like the idea of rooms arbitrarily placed, effectively making portals. I sense something involving a wardrobe soon 🙂

[BarDweller]

A room today is already in charge of handing /go

It does so by sending a request to GO to have GO switch the user in the direction the user asked for
now.. there's another message type (undocumented at the mo) which requests a teleport of the user to a destination room. The reason it's undocumented is that it was locked in the mediator only allow it to be processed if the source room was FirstRoom

This change unlocks that, so that it's allowed to be processed if the room isn't first room, provided the src & destination rooms have the same owner

So yes, if this goes in you could opt to handle /go in your rooms, by having it teleport the user to the next destination

Remembering that your rooms are scattered throughout the room grid, you may then want to do things like remember which players are part of your 'game' and anyone entering not via the proper entrance, (ie unknown players) gets teleported to your entrance room 😃

If you were truly cunning, you might also remember where the player came from when that happened, so that when they 'quit' your game, you can put them back there.

So many ways to handle this 😃

Eg. with a little thought you can offer unknown players a choice..

• 'pass through your room'
• 'join your game'
  where the first just lets users transit through your room, but not be 'part' of the room as if it were entered via your graph correctly.

Consideration may need to be made for the effect on Sweep/Interactive Map.. but we already "support" the idea of rooms messing with players by reversing /go directions etc, so this shouldn't be too evil =)

Mainly this should allow creation & import of more complex room graphs, advancing the need & requirement to solve cross-room item handling etc.

[pavittr]

Reviewed 2 of 4 files at r1.
Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @BarDweller)

---

mediator-app/src/main/java/org/gameontext/mediator/ClientMediator.java, line 168 at r1 (raw file):

                    teleport = message.getBoolean(FirstRoom.TELEPORT, false);
                }else{
                    // Not first room, but requesting teleport to non-null destination?

Think this works fine, although this method is becoming complex. I'd be happy to tick it off but perhaps we need the extra test to make sure we can confidently refactor in future?

---

mediator-app/src/main/java/org/gameontext/mediator/MapClient.java, line 176 at r1 (raw file):

Previously, ilanpillemer (Ilan Pillemer) wrote…

return getSite(roomId) == null ? null : site.getOwner();
would make easier on the eye?

+1

---

mediator-app/src/test/java/org/gameontext/mediator/ClientMediatorTest.java, line 309 at r1 (raw file):

    @Test
    public void testSwitchTeleportSameOwner(@Mocked RoomMediator srcRoom) throws Exception {

To avoid future regressions, do we need a test here for when ownerId is the same but it isn't a teleport?

[BarDweller]

Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @ilanpillemer and @BarDweller)

---

mediator-app/src/main/java/org/gameontext/mediator/ClientMediator.java, line 168 at r1 (raw file):

Previously, pavittr wrote…

Think this works fine, although this method is becoming complex. I'd be happy to tick it off but perhaps we need the extra test to make sure we can confidently refactor in future?

Yeah.. if this part became more complex we'd move the logic here out to a 'boolean isTeleportAllowed(...)' method and init the teleport var from that call. Not too bad at the mo.

---

mediator-app/src/main/java/org/gameontext/mediator/MapClient.java, line 176 at r1 (raw file):

Previously, pavittr wrote…

+1

Umm.. how does the 'site.getOwner()' get a non-null site to use?

---

mediator-app/src/test/java/org/gameontext/mediator/ClientMediatorTest.java, line 309 at r1 (raw file):

Previously, pavittr wrote…

To avoid future regressions, do we need a test here for when ownerId is the same but it isn't a teleport?

In theory if it's not a teleport it's covered by existing tests..

[pavittr]

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @ilanpillemer)

---

mediator-app/src/main/java/org/gameontext/mediator/MapClient.java, line 176 at r1 (raw file):

Previously, BarDweller (Ozzy Osborne) wrote…

Umm.. how does the 'site.getOwner()' get a non-null site to use?

couldn't it be

Site site = getSite(roomId);
return (site == null) ? null : site.getOwner();

?

---

mediator-app/src/test/java/org/gameontext/mediator/ClientMediatorTest.java, line 309 at r1 (raw file):

Previously, BarDweller (Ozzy Osborne) wrote…

In theory if it's not a teleport it's covered by existing tests..

Ok, that was my thought, I haven't looked at these tests before but I'm not seeing any other test being augmented to do srcOwner dstOwner type checks, my mock logic is hazy but I'm thinking that means those methods aren't being called in other tests otherwise the test would fail because an unexpected method would have been called? Or does our mocking library not work that way?

[pavittr]

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @ilanpillemer and @BarDweller)

[pavittr]

Reviewed 1 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @ilanpillemer and @BarDweller)

[BarDweller]

Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @ilanpillemer and @BarDweller)

a discussion (no related file):
Interesting thought.. if we do this.. we could/should also raise the game on protocol version to allow rooms to test if this protocol addition is accessible to them.

---