rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

[RFC] Prototyping speed improvements

Open AnneKitsune opened this issue 7 years ago • 31 comments

Problem

Prototyping in amethyst is slow.

100 lines of code seems to be the minimum to make the smallest of game. While its not a lot, it definitely is more than necessary.

Source

I'm taking https://github.com/amethyst/amethyst/blob/develop/examples/sphere/main.rs as a model Lack of good defaults for generic types. Imports Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed.

Propositions

Lack of good defaults for generic types.

Adding sensible defaults as type alias for generics.

  1. Add a default.rs file.
  2. Make a DefaultSomething type alias with a default consistent with the others.
  3. Export the module default.

Example: amethyst_input/src/default.rs

type DefaultInputHandler = InputHandler<String,String>

Imports

Expand the prelude to include as many common types as possible. I'm not too sure how much this slows down compile time when using the prelude, but I do have performance degradation in one of my project where I was using only preludes.

Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed.

I already implemented it inside of a custom project, currently debating if this should be inside of the engine.

AnneKitsune avatar Feb 26 '18 03:02 AnneKitsune

first!

AnneKitsune avatar Feb 26 '18 03:02 AnneKitsune

Bump !

majstudio avatar Feb 26 '18 03:02 majstudio

I'm fine with this, but please don't use external libraries which are unreleased (and not even close to complete) as the main example.

Rhuagh avatar Feb 26 '18 09:02 Rhuagh

Oops, wrong button.

Rhuagh avatar Feb 26 '18 09:02 Rhuagh

I mean, when is a normal user going to use something else, except in an extremely low amount of edge cases?

Just a personal opinion on this, but I would never use String.

Rhuagh avatar Feb 26 '18 09:02 Rhuagh

Not even for your prototypes?

AnneKitsune avatar Feb 26 '18 10:02 AnneKitsune

Not even for my prototypes.

Rhuagh avatar Feb 26 '18 11:02 Rhuagh

MaterialDefaults is a resource, I think it should be a constant.

No, it should not. It's a fallback for when a texture handle is invalid and the user may provide it. Also, a constant would need late initialization (and yeah involve global state, probably not work with multithreaded tests, ..).

torkleyy avatar May 19 '18 12:05 torkleyy

Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed. I already implemented it inside of a custom project, currently debating if this should be inside of the engine.

Thinking of adding it in amethyst_input/src/utils.rs instead of amethyst_utils. Would that make sense @torkleyy ?

ps: I removed the material defaults part.

AnneKitsune avatar May 19 '18 14:05 AnneKitsune

Sounds like an excellent location.

Rhuagh avatar May 19 '18 14:05 Rhuagh

Hmm I'm not sure, creating a convenience function for everything might obscure things too much.

torkleyy avatar May 19 '18 14:05 torkleyy

So, who wants to have this: https://github.com/jojolepro/amethyst-extra/blob/master/src/lib.rs#L193 (lines 193 to 257 included into the engine, and who doesn't? I don't mind keeping it external, but integrating it would make the examples cleaner.

AnneKitsune avatar May 19 '18 14:05 AnneKitsune

I agree there shouldn't be a function for everything, but this qualifies as extremely common. I say we should add it.

Xaeroxe avatar May 19 '18 14:05 Xaeroxe

Generating a quad and handling basic events should be part of the engine, I'd agree it's okay to add it. As long as we keep @torkleyy's concerns in mind for other (less integral) features.

MrMinimal avatar May 19 '18 14:05 MrMinimal

Well, I think it would be nice to have a more concise Event type. It should just be

match event {
    Event::KeyPressed(KeyCode::Escapce) => blah,
}

That would make things a lot nicer.

torkleyy avatar May 19 '18 14:05 torkleyy

That's why I'm still in favor of exposing our own event type instead of winit::Event.

torkleyy avatar May 19 '18 14:05 torkleyy

Why not expose both?

Xaeroxe avatar May 19 '18 14:05 Xaeroxe

Exposing just our own type allows us to upgrade winit without breakage. Additionally, if we convert winit events into our own type, I don't see any need to interface with the winit version.

torkleyy avatar May 19 '18 14:05 torkleyy

Having two ways of dealing with the problem would be confusing IMO.

torkleyy avatar May 19 '18 14:05 torkleyy

That's fair. I was mostly thinking of people who might want to use winit events to interface with other winit compatible crates, but I'm not sure I have any good examples at this time.

Xaeroxe avatar May 19 '18 14:05 Xaeroxe

I do that, I convert winit events myself into other events.

Rhuagh avatar May 19 '18 14:05 Rhuagh

I do think we can add a system event for the things that are hard events

Rhuagh avatar May 19 '18 14:05 Rhuagh

for the things that are hard events

What do you understand by that?

torkleyy avatar May 19 '18 14:05 torkleyy

Window close, resize etc, things that you usually don't rebind.

Rhuagh avatar May 19 '18 14:05 Rhuagh

I'm not sure I understand what you're suggesting, sorry.

torkleyy avatar May 19 '18 14:05 torkleyy

I'm not even sure I do myself :P

My current gripe with the current handle_event function calls are that there's absolutely no control from the users perspective what events get propagated there. For the most part if you want input in a usable manner today you'd request access to an EventChannel manually, and just ignore what comes to handle_event. Somehow I guess to basic use case for it was the ability to get easy access to input events.

The problem however is that input events can roughly be divided into two groups of events:

  • What I call hard events - Close, Resize, Focus etc - Things that are never rebound, just acted upon
  • Rebindable events - Mouse, Keyboard, Touchpad etc, i.e. actual input events, which are most likely passed through some kind of rebinding and transformed into an event structure specific to each game.

I'm not sure I even have a proposal right now, just some gripes.

Rhuagh avatar May 19 '18 20:05 Rhuagh

Is this issue up-to-date ? Are all listed problems still there?

raskyld avatar Oct 30 '18 19:10 raskyld

@AlmightyScientist Yes, this issue did not change a lot. I'll go through all the RFCs soon and make sure they're all discussed properly.

torkleyy avatar Oct 30 '18 21:10 torkleyy

Should we use the <T=ConcreteType> notation or use a typedef (type Default... = ...<ConcreteType>) ?

raskyld avatar Nov 02 '18 17:11 raskyld

<T=ConcreteType> where it makes sense to have a default type. For example, draw Passes shouldn't have a default for the data because there's no good default (PosTex, PosNormTex, PosNormTangTex, etc)

Those that don't should have a type Default..

AnneKitsune avatar Nov 03 '18 19:11 AnneKitsune