Better Portals compatibility

[Laike-Endaril]

See post in other issue here

[Laike-Endaril]

Pretty sure this will be fixable with a certain entity tracker compat/config system I have planned, but won't know for sure until that system is created.

[Laike-Endaril]

@Johni0702 Not sure if this ping will work since you haven't posted here yet, but I took a look at the timing on my replacement of the entity tracker, and I could see a few things that happen sooner where another mod could possibly grab a reference to the original tracker before it gets replaced.

I'll go ahead and look into this one more today, since I think it may end up being a simpler fix than expected

[Laike-Endaril]

Looks like I didn't miss any references...in fact, there is only one field in MC where a tracker is stored (which I'm overwriting the value of). You must be grabbing and storing a reference to it before my replacement occurs. I'll look into timings.

[Johni0702]

Pings work everywhere, regardless of whether the target has already participated in the issue.

I'm not sure what you mean by timing on my replacement of the entity tracker. The time at which you're replacing the vanilla entity tracker is fairly irrelevant to this issue as long as it happens before the first player connects (but if it didn't you'd already have internal conflicts yourself).

The compatibility issue with BP arises because BP uses access transformers to make EntityTracker.entries public. At that point, two entries fields exist in your entity tracker with the same name (though not technically the same name, the original one will actually be remapped to its srg name field_72793_b). So, BP uses access transformers to modify field_72793_b whereas your EntityTrackerEdit uses its own, distinct entries set.

One simple fix would be for your EntityTrackerEdit.entries to point to the same set as field_72793_b by retrieving the original set from the base class via reflection instead of creating a new one. That'll work as long as no one is replacing the set in field_72793_b (BP doesn't, it just modifies it).

[Laike-Endaril]

Ah, I didn't think of the possibility of access transformers being used from another mod...though I'd think my own entity tracker would still reference its own fields anyway? I'll look into that as well though, especially if you don't store any EntityTracker references in fields...

Do you store any references?

[Johni0702]

Yes, your EntityTrackerEdit references EntityTrackerEdit.entries which is distinct from EntityTracker.field_72793_b (which any other mod using ATs will access).

I don't think BP's storing any permanent references to that set or any EntityTracker instances. I don't think it should matter though.

[Johni0702]

FYI, this is the relevant BP code, roughly translated to Java:

    @Override
    public Consumer<EntityPlayerMP> swap(EntityPlayerMP prevPlayer) {
        List<EntityTrackerEntry> knownEntities = new ArrayList<>();
        for (EntityTrackerEntry entry : prevPlayer.getServerWorld().getEntityTracker().entries) {
            if (entry.trackingPlayers.remove(prevPlayer)) {
                entry.getTrackedEntity().removeTrackingPlayer(prevPlayer);
                knownEntities.add(entry);
            }
        }
        return (newPlayer) -> {
            for (EntityTrackerEntry it : knownEntities) {
                it.trackingPlayers.add(newPlayer);
                it.getTrackedEntity().addTrackingPlayer(newPlayer);
            }
        };
    }

[Laike-Endaril]

Yeah, it doesn't look like you have any stored references (as far as I can tell, at least, from what you posted above plus a search-in-files I did).

Also, I get what you're saying now; I made my edited entity tracker before I even knew ATs existed, so I did it via the new data structure + copy values from old one (as you said). I'll probably change that to an AT and see if it fixes the issue.

[Laike-Endaril]

Or rather, I'm not sure I copied anything even, because it would always be empty at the time of creation/replacement anyway, but still, yes, changing to AT should allow other mods to reference the field and be compatible with my tracker

[Laike-Endaril]

There's one other possible issue though; if you're adding tracker entries directly to the data structure instead of the normal methods, it will bypass my checks and won't create the edited version of a tracker entry I created for living entities. This will probably not crash, but will probably make all entities created this way fully visible at all times. I'll have to check to see if that happens or not.

Basically, it might be "more correct" for me not to use an AT here, because other mods should probably not have direct access to that data structure unless they know how to create an instance of my LivingBaseEntityTrackerEntry...unless I find another way to accomplish that, which probably would involve ASM at that point.

Do you need to access the data structure directly, or can your goal be accomplished by using the normal EntityTracker methods?

[Laike-Endaril]

Note: I was assuming that you'd be creating entries directly, but actually, so long as the entries are created via the normal methods, swapping them in and out will probably work fine...I'll just go test it

[Johni0702]

I'm swapping two player entities (one fake, one real) between two worlds without re-sending all the data. The reason why I need to do that is because I do the same thing client-side and also swap out the connection associated with each of the entities. This allows BP to have the player switch dimension without any loading screen or (in the particular case of the entity tracker) without re-spawning entities i.e. practically seamless (otherwise all entity data would be re-sent and any client-side interpolation be reset). There's no reliable way to do that without accessing internals.

[Laike-Endaril]

Righto. I'll have to test to see if it breaks player <-> player visibility then. So far player -> npc visibility is working fine and portals are no longer broken...I'll run more tests

[Laike-Endaril]

I take it back; it looks like player -> NPC visibility is broken, angle-wise. I assume this means the "swapped out" player entity is not getting its look direction synced to that of the actual player? If so, you'll have to look into that part from your end. I'll test player <-> player visibility next

[Laike-Endaril]

Player <-> player visibility is "fine" as well.

Ie. same as player -> NPC visibility; needs an angle sync, but otherwise seems to be working.

I'll probably release an update for DS within 10 mins with the fixes for my end.

[Laike-Endaril]

30 mins*

[Johni0702]

There's no syncing of the fake player at all. In fact, the same fake player may be shared by multiple portals. I'm not yet sure what to make out of view sharing because it has various other difficult to deal with problems anyway, yet it improves performance by such an amount that I really do not want to get rid of it. Nothing really you need to worry about though.

If it's only broken in the remote world (i.e. looking through a portal) but starts working fine once you step through the portal, then I wouldn't consider that a bug for now.

[Laike-Endaril]

Yeah, that's the gist of it; entities on the other side of the portal are seen as if from whatever angle the "other" player entity is at. Not a huge bug; certainly better overall than it was before.

[Laike-Endaril]

Alright, this is now mostly fixed in the latest release of DS (087).

Loose ends are... ...angle thing I mentioned. ...possibly entity -> player visibility...did not check that. Ie. NPCs/mobs may not correctly see players on the other side of a portal (did not check). Possibly exploitable if so (for PvE).

That said, will probably close for now and just put a note in my local TODO

[Laike-Endaril]

(Yes, entity -> player visibility is broken...but right now that happens even without DS installed)

[Johni0702]

Correct, for now that's an issue in BP, it doesn't yet touch the entity AI at all: https://github.com/Johni0702/BetterPortals/issues/63

[Laike-Endaril]

Got it, thanks for the info