WheteThunger / MonumentAddons

Add entities, spawn points and more to monuments
https://umod.org/plugins/monument-addons
MIT License
5 stars 9 forks source link

Move spawn point when moving entity spawned by it #8

Closed WheteThunger closed 2 years ago

WheteThunger commented 2 years ago

Currently, it's not possible to move a spawn point with telekinesis. We should make it so that when moving an entity spawned by a spawn point, that upon releasing Telekinesis, it moves the spawn point to the entity's location.

WheteThunger commented 2 years ago

This should probably be implemented in the OnTelekinesisStopped hook, for which the current implementation is below.

private void OnTelekinesisStopped(BasePlayer player, BaseEntity moveEntity, BaseEntity rotateEntity)
{
    SingleEntityAdapter adapter;
    SingleEntityController controller;

    if (!_entityTracker.IsMonumentEntity(moveEntity, out adapter, out controller))
        return;

    if (!adapter.TrySaveAndApplyChanges())
        return;

    if (player != null)
    {
        _adapterDisplayManager.ShowAllRepeatedly(player);
        ChatMessage(player, LangEntry.SaveSuccess, controller.Adapters.Count, controller.Profile.Name);
    }
}

I think the _entityTracker.IsMonumentEntity method is returning false because it only checks if the entity belongs to a SingleEntityAdapter. I don't think we should alter the implementation of _entityTracker.IsMonumentEntity, but rather, we should perform a separate check in the OnTelekinesisStopped method to check if the entity belongs to a spawn point. There's already a helper method for this called GetSpawnPointAdapter(BaseEntity).

m-hynek commented 2 years ago

I tried to solve this one, but I dont know where to start. I've come up with this code, it works most of the time, but

// This hook is exposed by plugin: Telekinesis.
private void OnTelekinesisStarted(BasePlayer player, BaseEntity moveEntity, BaseEntity rotateEntity)
{
    if (_entityTracker.IsMonumentEntity(moveEntity) || GetSpawnPointAdapter(moveEntity) != null)
        _adapterDisplayManager.ShowAllRepeatedly(player);
}

// This hook is exposed by plugin: Telekinesis.
private void OnTelekinesisStopped(BasePlayer player, BaseEntity moveEntity, BaseEntity rotateEntity)
{
    SingleEntityAdapter adapter;
    SingleEntityController controller;

    int adapterCount;
    string profileName;

    var spawnPointAdapter = GetSpawnPointAdapter(moveEntity);

    if (spawnPointAdapter != null)
    {
        BaseMonument monument;
        if (!VerifyAtMonument(player.IPlayer, moveEntity.transform.position, out monument))
            return;

        Vector3 localPosition;
        Vector3 localRotationAngles;
        bool isOnTerrain;

        DetermineLocalTransformData(moveEntity.transform.position, player, monument, out localPosition, out localRotationAngles, out isOnTerrain);

        spawnPointAdapter.SpawnPointData.Position = localPosition;
        spawnPointAdapter.SpawnPointData.RotationAngles = localRotationAngles;
        _profileStore.Save(spawnPointAdapter.Profile);

        spawnPointAdapter.ProfileController.Reload(spawnPointAdapter.Profile);

        adapterCount = spawnPointAdapter.Controller.Adapters.Count;
        profileName = spawnPointAdapter.Profile.Name;
    }
    else if (_entityTracker.IsMonumentEntity(moveEntity, out adapter, out controller))
    {
        if (!adapter.TrySaveAndApplyChanges())
            return;

        adapterCount = controller.Adapters.Count;
        profileName = controller.Profile.Name;
    }
    else
        return;

    if (player != null)
    {
        _adapterDisplayManager.ShowAllRepeatedly(player);
        ChatMessage(player, LangEntry.SaveSuccess, adapterCount, profileName);
    }
}
WheteThunger commented 2 years ago

sometimes VerifyAtMonument(player.IPlayer, moveEntity.transform.position, out monument) returns false even when I was deep whitin monument bounds and I didnt figure out why (tested with scrap heli on powerplant)

Sometimes I have seen a height issue. If the monument bounds end right at the terrain level, sometimes the check thinks the ground is out of bounds. If this is the issue, should be able to fix by slightly increasing bounds in Monument Finder.

reloading profile is bad

Agreed. This is the most involved part of this change. It requires introducing methods at several levels.

Improvements can be identified once you have a base that does the above. Arguably the UpdateSpawnGroups method could be the place to put this. Consolidating all types of changes into a single fan-out method is the direction I ended up refactoring toward for SingleEntityController (handleChanges).

Plugin should respawn it from updated profile position after telekinesis stopped, so admins can see exact respawn position.

Agreed, this is the approach I've tended to follow and it's worked out well.

From plugin functionality i understand that only Y rotations are allowed for spawnpoints

I think that's just when creating them. We should make it possible to rotate spawn points on all axes since some vanilla ones are rotated on X/Z (road signs).

when using /tls on entity with rigidbody, it tends to rotate 'randomly'

We can probably solve this by temporarily setting the rigidbody to kinematic. Take a look at the RigidbodyRestorePoint class in that plugin. Just need to add another property to track the isKinematic field, and set it to true initially. Like this:

private class RigidbodyRestorePoint
{
    private Rigidbody _rigidBody;
    private bool _useGravity;
    private bool _isKinematic;

    public static RigidbodyRestorePoint CreateRestore(Rigidbody rigidbody)
    {
        if (rigidbody == null)
            return null;

        if (!rigidbody.useGravity && rigidbody.isKinematic)
            return null;

        var restore = new RigidbodyRestorePoint
        {
            _rigidBody = rigidbody,
            _useGravity = rigidbody.useGravity,
            _isKinematic = rigidbody.isKinematic,
        };

        rigidbody.useGravity = false;
        rigidbody.isKinematic = true;

        return restore;
    }

    public void Restore()
    {
        if (_rigidBody == null)
            return;

        _rigidBody.useGravity = _useGravity;
        _rigidBody.isKinematic = _isKinematic;
    }
}

One note is that we should verify the monument the entity was moved to is the same exact monument of the spawn point. Can probably achieve this by just checking whether the new position is within the bounds of the monument already associated with the SpawnGroupAdapter, rather than by doing the VerifyAtMonument call.