feat: block player movement

[AlejandroAlvarezMelucciDCL]

This PR is part of Cycle's 8 Shape Feature block player movement

Related PRs:
SDK: #991
Explorer: #1777

[github-actions]

Test this pull request

• The @dcl/protocol package can be tested in scenes by running

  npm install "https://sdk-team-cdn.decentraland.org/@dcl/protocol/branch//dcl-protocol-1.0.0-10560357685.commit-51e7c06.tgz"

[gonpombo8]

wasn't this suppose to be a bit operator ?
so if you want to disable the walk & run you can jsut do standard: SI_WALK | SI_RUN ?

[AlejandroAlvarezMelucciDCL]

The first iteration was to treat this as layers, yes, but then we moved to the protocol-squad approach, which uses oneof letting us to expand this component in the future. You can combine disables just fine like this:

InputModifier.createOrReplace(engine.PlayerEntity, {
    mode: {
      $case: "standard", standard:
      {
        disableWalk: true,
        disableJump: true,
      }
    }
  })

And the plan is to implement a helper function, probably during hardening week.

We could also remove the oneof altogether if we don't want to expand this component with different modes, what do you think?

[gonpombo8]

mmm i didnt get it. You can still have the oneof approach with the bitwise. 🤔

[AlejandroAlvarezMelucciDCL]

Yeah sorry I didn't explained better 🙏
I liked this approach better because it doesn't use bitwise, I think it's more complex for creators and prone to errors. A week or two ago, I spotted a bug on the NFT Museum where the logical OR (||) was used instead of the bitwise OR (|) which results in different bitwise values.
So this approach removes that complexity and it's super clear what are you disabling.

If you have arguments in favor of bitwise and against this approach please share them since I'm not an expert on these tech-stack so maybe there's more to it that I'm not aware of 💪

[AlejandroAlvarezMelucciDCL]

@leanmendoza do you mind jumping in?

[pravusjif]

Uhmmm I would remove the oneof since I can't imagine which "non standard" modes we could have.

And regarding the bitwise vs bools, I agree for CRDT Messages that have to be as small and fast as possible having bits instead of bytes would consume less memory (and checked faster in the client side), it can be argued how negligible those gains are in a desktop application but even so I prefer bitwise in this case.

We could have an enum called something like InputType with values like IT_ALL, IT_JUMP etc. and something like uint32 blocked_input_mask as the mask

[leanmendoza]

Regarding this protocol change, I talked a lot with Nico and Ale about how we interact with the protocol and how this affects creators. Sorry if this doesn't specifically answer what is being discussed here, but it's a good place to have the conversation. These are the ideas I'm exploring so far:

1. Don't limit ourselves to using the tools that Protobuf provides due to poor exposure to the content creator: this is about oneof, which may seem horrible, but it's ideal for cases where you have to work with exclusion (by definition, in the protocol)
2. Don't limit ourselves to using Typescript's tools to make the SDK user experience more pleasant. It's time to start exploring more specific solutions (e.g. doing a remap in the Material with defineProperty) so that the autogeneration done from the protocol is not what is exposed to the user. We have already tackled this with helpers, but it can be further explored. (open to debate)
3. Weigh the messages in relation to their use and not in general. If a message has a continuous modification (such as a Transform, Audio Volume, etc.), we may want to save bandwidth there, but that's not the case for this message. And if we consider that when boolean=false, the protobuf serialization does not compute; so if every value is false, the message is length=0, it even makes more sense.

In this particular case, for the InputModifier, there may be other modifiers (such as forces for different actions, remap of inputs, left/right rotates avatar instead of walk, left/right walks with left/right steps instead of forwad), this is really up to you. I told Ale that in the worst case other components can be built (v2,v3,v4) when one becomes deprecated.
As for the mask, for usability, we have talked about it with Nico, and based on the creators' experience, it is more intuitive to have several booleans. But if we invoke points 1 and 2, this could be a mask at the protocol level with booleans exposed to the user, and this is what I'd like to go for.

[gonpombo8]

Agree, and what we expose to the user's doesnt have to limit how we implement it on the background.
We are doing something similar on the UI also with tons of helpers so it's easier for the content-creator.

But i would avoid to get the same results for a problem, with diferente solutions. If for the same use case we are using bitwise, why not here ? Then if the content-creator wants helpers because Nico think's its a better idea we can implement them. (Also in the future this things would be a checkbox on the editor)

[robtfm]

for high frequency messages like transforms it's important to be optimal. for lower frequency messages, i think the clarity and easy implementation from separate fields is more important.

i don't think the existing bitfields should stop the protocol from choosing a different priority for new messages.

[AlejandroAlvarezMelucciDCL]

Thanks everyone for participating in this debate, I'm not sure I fully understand every point stated here 😅 but I'm confident on all of your judgements and I'm glad we found an agreement! I'll merge this PR as is but feel free to spark a new discussion if needed, we can always introduce a new version of this component like Lean mentioned.