capnmidnight/Primrose

Create configuration parameter for defining a model to replace the teleport pad.

Closed this issue · 6 comments

capnmidnight commented

The teleport pad is currently a flat circle with a flat color applied to it. There should be an option to change that to an arbitrary model loaded from a file.

lougeniaC64 commented

@capnmidnight I'll take a stab at this one.

lougeniaC64 commented

Okay, @capnmidnight, I've got a few (possibly newb) questions.

  • Should the teleport pad model be 2d or 3d (wondering if it needs to be a Three.geometry), or should either be possible?
  • I'm assuming the model file should be stored in the /shared_assets/images/ directory, but wanted to check first. Did you have something else in mind for how to load the model file?
  • This might be a reworded version of my first question, but should the Teleporter class have the same function properties (_exit, _start, _move, etc.) as the flat circle?
capnmidnight commented
lougeniaC64 commented

No rush, @capnmidnight. If it's not tomorrow, no worries. I'll be here.

capnmidnight commented

You should be updating the current Teleporter class, not making a new one. The current Teleporter class needs functionality to change how it looks, depending on what options the user provides. Currently, it's the flat disk (https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/Tools/Teleporter.js#L42-L53) (the boundingBox calculation there is just to move the default cylinder shape that centers on zero to be completely above zero in the Y axis).

The model file should be loadable from anywhere. This will be configured as an option when creating a BrowserEnvironment. The documentation for BrowserEnvironment's configuration parameters is here: https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/BrowserEnvironment.js#L19-L199

You'd add a new field teleportPadModel to these options. Like the groundModel option, it should be a String. I think it should be null by default (BTW, the default values are set here: https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/BrowserEnvironment.js#L1947-L2006).

You should create a new demo in the demos directory to exercise the option. Just copy the "empty" demo, name it "custom_teleport" or something, and add the config parameter. Yes, you should store the model file for the demo in the demos/shared_assets/models directory. I suspect you don't have a model as of yet, so I made one that you can use if you don't want to bother installing Blender: demos/shared_assets/models/teleport.obj.

The Ground object goes through a process to figure out what to show for the ground based on the user input: https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/Controls/Ground.js#L41-L69. The Teleporter should do a similar thing.

If the teleportPadModel value is null, then the current circle pad that is being used should be created as the teleport model.

You don't have to support a Number value to change the color of the teleport pad if you don't want to, but it'd be a nice to have.

You also don't have to support an image file for texturing the default teleport pad. It doesn't exactly make sense. If the user really wants to have a custom texture on a teleport pad, they could always just embed it in a model. Also, there's no good way to tell if a particular string is an Image file or a Model file before attempting to load it, so there'd have to be two separate configuration parameters (like how there is a separate groundTexture and groundModel parameter. In the case of the ground, there is a significant use case for having both. But for the teleport pad, I don't really see using an image as being that common.

Just like in the Ground class, you should do the construction of the model in an accessor called _ready: https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/Controls/Ground.js#L32-L92, returning a promise from the model loader that fulfills when the model has finished loading. We'll need that promise in the BrowserEnvironment to know when the scene has finished loading: https://github.com/capnmidnight/Primrose/blob/master/src/Primrose/BrowserEnvironment.js#L1663

Ground inherits from Primrose/Controls/Entity, which has a property called ready (no underscore) that looks for the property _ready (with underscore). Any callees using an Entity should access ready and not _ready. Fields with underscores are considered "private" to those classes. In this case, the public version sits, waits for the promise, drops whatever the promise actually returns, and forces a reference to the Entity to be returned instead.

Teleporter is currently in Primrose/Tools and does not inherit from Primrose/Controls/Entity. Move it to Primrose/Controls and delete the Primrose/Tools directory. You'll have to update the import statements in Primrose/BrowserEnvironment (to change Teleporter from Tools to Controls), Primrose/Controls/index.js (to add Teleporter), and Primrose/index.js (to remove Tools).

Otherwise, the Teleporter class should stay the way it is. Those _exit, _start, _move functions are for interacting with the event system, so they need to stay where they are. I've maybe made this sound a lot more complicated than necessary. Essentially, the steps are:

  • move the Teleporter class from Tools to Controls, and update the references,
  • make Teleporter inherit from Entity, looking to Ground for guidance.
  • move the creation of the disk field to a property getter called _ready
  • add logic within _ready to choose what geometry to show for disk given the user's options
  • make _ready return a promise that is either already resolved if the default disk is being used or is the promise that is returned from ModelFactory.loadObject if the "disk" is coming from a model file.
  • make BrowserEnvironment wait on that promise as it already does for the Ground.
  • 👍1
lougeniaC64 commented

Thanks for all the info, will get to work!