tresinformal / drakkar

The tresinformal video game called 'Drakkar'
GNU General Public License v3.0
11 stars 4 forks source link

[MVG] Remove key bindings for shooting rocket and stun rocket #644

Closed EvoLandEco closed 1 year ago

EvoLandEco commented 1 year ago

To make a minimal viable game (MVG), we have to implement and also remove some functionalities.

We will remove rocket and stun rocket from the game by commenting out the key bindings. We think that this approach would be easier and has less impact on the whole program.

EvoLandEco commented 1 year ago

As @richelbilderbeek suggested, a good approach would be to write a function to create a custom/test key action map for the minimal viable game purpose.

EvoLandEco commented 1 year ago

Or, we could map the keys to shoot/stun to the action type "none"

richelbilderbeek commented 1 year ago

Hi @EvoLandEco: using a none type is nearly always a bad idea, as every switch statement has to deal with it. This means this architecture scales badly. Returning an optional without a value in the few functions this is needed is superior.

EvoLandEco commented 1 year ago

Hi @richelbilderbeek , could you please elaborate a bit more about optional? Do you mean std::optional?

TheoPannetier commented 1 year ago

I think @richelbilderbeek is referring to some kind of specific type that exists in cpp? @EvoLandEco was referring to action_type::none (one of the possible values of enum class action_type). Linking the keys to this value would result in these keys not triggering any action. @richelbilderbeek was there a misunderstanding here, or do you reckon this course of action would be a bad idea?

TheoPannetier commented 1 year ago

cf our conversation in #648

richelbilderbeek commented 1 year ago

@EvoLandEco well spotted, we model optional after std::optional. We wrote our own optional, as some time ago we used C++11, at which std::optional was not there yet (?maybe as std::experimental::optional?). Also, it was a good exercise!

If you can replace the code in optional.h to using optional = std::optional; would be great if that would work :-)

richelbilderbeek commented 1 year ago

@TheoPannetier yes, optional/std::optional is standard C++. A std::optional is basically a std::vector that has either zero or one element. It allows you to 'return nothing', instead of adding an extra enum class value (i.e. none).

EvoLandEco commented 1 year ago

@EvoLandEco well spotted, we model optional after std::optional. We wrote our own optional, as some time ago we used C++11, at which std::optional was not there yet (?maybe as std::experimental::optional?). Also, it was a good exercise!

If you can replace the code in optional.h to using optional = std::optional; would be great if that would work :-)

Could we just use std:optional without using? Some consider using namespace std a bad practice, I suppose using optional = std::optional would be somewhat similar?

EvoLandEco commented 1 year ago

std::optional: How, when, and why, an article that might be useful, for reference

EvoLandEco commented 1 year ago

@richelbilderbeek I re-examined the code and I suppose there might still be some misunderstandings, I was planning to map the two key bindings to action_type::none, and this action type has long been existed in the code, I interpreted it as an idle status, thus I believed that re-assigning the key bindings from shoot and shoot stun rocket to none/idle was legit? Perhaps we can change none to idle here?

richelbilderbeek commented 1 year ago

Hi @EvoLandEco ,

Could we just use std:optional without using? Some consider using namespace std a bad practice, I suppose using optional = std::optional would be somewhat similar?

Yes, using namespace std in a header file is a bad idea (due to Koening lookup), using optional = std::optional can be useful while transitioning to C++17.

I was planning to map the two key bindings to action_type::none, and this action type has long been existed in the code, I interpreted it as an idle status, thus I believed that re-assigning the key bindings from shoot and shoot stun rocket to none/idle was legit? Perhaps we can change none to idle here?

I think it is a bad idea to map a key to doing nothing, whether it is called idle or none, as, well, you program how to do nothing...?

Still, I am happy to have my mind changed: show me some code that really needs this :-) . I predict, however, there is none.

EvoLandEco commented 1 year ago

closed by #648