Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow sending spawnData when spawning entities using the MultiplayerService #1400

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

NotBjoggisAtAll
Copy link

An attempt at resolving my own question on the discussion page.

Tested with a local project where I first encountered the need for this.

@AlmasB
Copy link
Owner

AlmasB commented Jan 22, 2025

Thanks for this. The public API seems fine. However, implementation-wise, SpawnData can carry non-serializable data, so I'm just a bit cautious it may break compatibility with existing projects and/or cause confusion regarding the contractual obligation of SpawnData.

Is there a way we can achieve the same without making SpawnData serializable? Perhaps some internal or public data structure (which is serializable) that can carry the spawn data, NetworkSpawnData extends Serializable, which contractually forces developers to keep only Serializable data in it, possibly a serialization friendly version of PropertyMap.

Another aspect we need to try and figure out is what to do when SpawnData does carry non-serializable data. Using NetworkSpawnData from above may solve this. I'm curious to know what your thoughts are on this.

@NotBjoggisAtAll
Copy link
Author

I agree with your concerns about changing SpawnData serializable,
I can make a NetworkSpawnData that is serializable instead. I think it should be public so that others can make subclasses to provide more data to send from the server to the client.

I think it should be up the the users to make sure all the data is serializable if they choose to create a subclass of NetworkSpawnData

Does this sound okay? I can make the changes and then we can see if its enough

@NotBjoggisAtAll
Copy link
Author

Where do you want the NetworkSpawnData class to live? Under entity like with SpawnData or together with the MultiplayerService under the multiplayer package?

@AlmasB
Copy link
Owner

AlmasB commented Jan 24, 2025

Probably alongside MultiplayerService, we can change later as needed. Above all sounds good.

@AlmasB
Copy link
Owner

AlmasB commented Jan 24, 2025

Not sure if helpful, but perhaps you can make use of Bundle to keep the data inside NetworkSpawnData. Bundle is fully serializable and is what's used when saving to the file system

@NotBjoggisAtAll
Copy link
Author

I wonder if NetworkSpawnData should extend SpawnData since the GameWorld.spawn() method expects a object of type SpawnData. Otherwise I would need to find a way to convert between the two SpawnData classes

@AlmasB
Copy link
Owner

AlmasB commented Jan 24, 2025

I'd go with a constructor for now NetworkSpawnData(SpawnData)

@NotBjoggisAtAll
Copy link
Author

I tried to do something like this but my demo fails since SpawnData is not serializable.

open class NetworkSpawnData(val spawnData: SpawnData) : Serializable {
}
-----------------

    fun spawn(connection: Connection<Bundle>, entity: Entity, entityName: String, spawnData: SpawnData) {
        if (!entity.hasComponent(NetworkComponent::class.java)) {
            log.warning("Attempted to network-spawn entity $entityName, but it does not have NetworkComponent")
            return
        }

        val networkComponent = entity.getComponent(NetworkComponent::class.java)

        val event = EntitySpawnEvent(networkComponent.id, entityName, NetworkSpawnData(spawnData))

        // TODO: if not available
        val data = replicatedEntitiesMap[connection]!!
        data.entities += entity

        fire(connection, event)
    }

----------------------
Demo

     // This does not work.
      SpawnDataspawnData = new SpawnData(x, y);
      Entity entity = spawn("test", spawnData);
      getMultiplayerService().spawn(connection, entity, "test", spawnData);

      //This work because I send my custom spawn data class which implements Serializable
      CardSpawnData cardSpawnData = new CardSpawnData(x, y, card.suit(), card.rank());
      Entity entity = spawn("card", cardSpawnData);
      getMultiplayerService().spawn(connection, entity, "card", cardSpawnData);
   
      

I could try to use a Bundle somehow, but I'm not too sure on how to use it. I could also set data inside SpawnData x,y,z,data manually on the NetworkSpawnData, but then I'm not sure to get custom variables/fields that exists on subclasses of SpawnData

@AlmasB
Copy link
Owner

AlmasB commented Jan 24, 2025

NetworkSpawnData(val spawnData: SpawnData) -- the val there makes spawnData part of the class. Perhaps get rid of val and parse / convert all you need from spawnData's data object into NetworkSpawnData

@AlmasB
Copy link
Owner

AlmasB commented Jan 24, 2025

I was thinking something like this.

NetworkSpawnData(spawnData: SpawnData) {

    val bundle = Bundle("NetworkSpawnData")

    init {
        spawnData.data.forEach((key, value) -> bundle.put(key, value));
    }
}

Not sure what we should do if value is not serializable. I guess for now you could check if it is, then put(), otherwise ignore.

Once we get NetworkSpawnData through the network to the other end, we need to reconstruct local SpawnData, which we should be able to do via the bundle object and its data field. Let me know if this makes sense.

Incidentally, I don't know/remember why SpawnData is an open class -- its internal Map should be able to carry anything, meaning we can potentially remove the open modifier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants