amethyst / rfcs

RFCs are documents that contain major plans and decisions for the engine
Apache License 2.0
32 stars 10 forks source link

[RFC] Prototyping speed improvements #18

Open AnneKitsune opened 6 years ago

AnneKitsune commented 6 years ago

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 commented 6 years ago

first!

majstudio commented 6 years ago

Bump !

Rhuagh commented 6 years ago

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 commented 6 years ago

Oops, wrong button.

Rhuagh commented 6 years ago

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.

AnneKitsune commented 6 years ago

Not even for your prototypes?

Rhuagh commented 6 years ago

Not even for my prototypes.

torkleyy commented 6 years ago

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, ..).

AnneKitsune commented 6 years ago

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.

Rhuagh commented 6 years ago

Sounds like an excellent location.

torkleyy commented 6 years ago

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

AnneKitsune commented 6 years ago

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.

Xaeroxe commented 6 years ago

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

MrMinimal commented 6 years ago

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.

torkleyy commented 6 years ago

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 commented 6 years ago

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

Xaeroxe commented 6 years ago

Why not expose both?

torkleyy commented 6 years ago

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 commented 6 years ago

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

Xaeroxe commented 6 years ago

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.

Rhuagh commented 6 years ago

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

Rhuagh commented 6 years ago

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

torkleyy commented 6 years ago

for the things that are hard events

What do you understand by that?

Rhuagh commented 6 years ago

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

torkleyy commented 6 years ago

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

Rhuagh commented 6 years ago

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:

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

raskyld commented 6 years ago

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

torkleyy commented 6 years ago

@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.

raskyld commented 6 years ago

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

AnneKitsune commented 6 years ago

<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..

fhaynes commented 5 years ago

Transferring to RFC repo