fernandojsg / aframe-input-mapping-component

https://fernandojsg.github.io/aframe-input-mapping-component
MIT License
24 stars 7 forks source link

Specify "default" mapping instead of using it as its key #15

Closed fernandojsg closed 6 years ago

fernandojsg commented 6 years ago

Currently we've a default mapping that will be applied by default. Although it prevent that default state from having a meaningful name and makes it hard to modify the default state when debugging or modifying the mapping. I prefer to use the default attribute to define the key of the default mapping.

We could go from

mappings: {
  default: {
    },
   grabbing: {
   }
}

To something like:

default: 'paint',
mappings: {
  paint: {
    },
   grabbing: {
   }
}
dmarcos commented 6 years ago

What happens if default is not defined? What about keeping the same structure we had? maybe a mapping can reuse other mappings and just express the differences and default is not an exception.

mappings: {
   {
      default : { use: ‘paint’ },
      grabbing: { ... },
      paint: {...}
   }
}
fernandojsg commented 6 years ago

I prefer to make it more verbose, having a use attribute seems handy when you're defining the mappings by hand, but hard to manage if you're creating them with an UI, so by now I'll stick to duplicate if we need to.

Based on the https://github.com/fernandojsg/aframe-input-mapping-component/issues/14 discussion where we have the inputActions exposed by the app and inputMappings defined by user, UI, or so. It makes sense that the default state is not part of the inputMappings as is user defined but the state should be app defined.

I believe the correct way to handle it should be to manually set the default state by the app, not requesting the user to do so. If no default state defined, it will just throw an error as we can't just rely on choosing the first one from the list. Maybe an optional parameter when registering the input actions or just by manually setting the currentInputMapping variable.

netpro2k commented 6 years ago

I agree that we probably dont need to build in re-use. For manually defining mappings you can just use normal javascript constructs to do this, and for auto-generated configs, its more simple to just duplicate information.

Re default state, users never directly set the current input state, they only define the mappings for a given state. I agree that we can just require the developer to set this initial state though we should probably call it something like currentInputState or currentActionSet instead of currentInputMapping to start to develop some more consistent naming.

fernandojsg commented 6 years ago

@netpro2k yep, I've pushed a PR with the modiciations: https://github.com/fernandojsg/aframe-input-mapping-component/pull/20 Regarding the naming, I'll open another issue to discuss the naming and then I could refactor it with a consistent naming everywhere (https://github.com/fernandojsg/aframe-input-mapping-component/issues/23)