Conversation
-Added a HashMap implementation in common, and implemented in some places -Now, Console UI Theme tooltips will be accurate -Now, sneaking with the controller will be toggleable -Now, the UI Theme will be applied when exiting the options screen -Now, the font rendering will be faster -Now, Vertical Layout will always navigate to the next element when scrolling down -Fixed Skeleton arrow shooting not accounting the head height -Fixed item swapping not calling slot changed, not changing the crafting result -Fixed Vertical Layout labels being selectable -Fixed not being able to navigate if there aren't selected elements
| XINPUT_STATE& inputState = m_inputStates.m_inputState[id]; | ||
| bool joinGameAlreadyFired = false; | ||
| for (ButtonIDMap::const_iterator it = m_buttonIdMap.begin(); it != m_buttonIdMap.end(); it++) | ||
| for (ButtonIDMap::Iterator it = m_buttonIdMap.begin(); it != m_buttonIdMap.end(); it++) |
There was a problem hiding this comment.
I'd really prefer that our HashMap class have the same general interface semantics as std::unordered_map
There was a problem hiding this comment.
So this means Iterator being iterator it.key() being it->first, etc. This will make it way easier to quickly change std::maps to hash maps.
| { | ||
| switch (button) | ||
| { | ||
| case SDL_CONTROLLER_BUTTON_A: |
There was a problem hiding this comment.
Could you make the case return take up one line?
| } | ||
| } | ||
|
|
||
| GameController::EngineButtonID AppPlatform_sdl::GetEngineButton(uint8_t button) |
There was a problem hiding this comment.
This could just be _getEngineButton, and only exist in the CPP file. No need to declare it in the header or as static and whatnot.
| bool Minecraft::isTouchscreen() const | ||
| { | ||
| return m_bIsTouchscreen; | ||
| return IInputHolder::activeType == IInputHolder::TOUCHSCREEN; |
There was a problem hiding this comment.
This should really be a static getter somewhere worst-case.
There was a problem hiding this comment.
Also, I'm not a fan of having the enum inside of IInputHolder, since we've never put enums in interfaces before. Additionally, this makes it difficult to tell what exactly the enum is representing. Is it an input holder type?
There was a problem hiding this comment.
That's temporary, my plan is making this non-static for the splitscreen (member of Minecraft), but this will need other changes, considering Mouse, Keyboard and GameControllerManager are static.
But yeah, it's an input holder type for now, I wouldn't change this while splitscreen isn't a thing, do u really want to make changes?
| bool Minecraft::useController() const | ||
| { | ||
| return m_pPlatform->hasGamepad() && getOptions()->m_bUseController.get(); | ||
| return m_pPlatform->hasGamepad() && (IInputHolder::activeType == IInputHolder::CONTROLLER || getOptions()->m_bUseController.get()); |
There was a problem hiding this comment.
Do we even need to check AppPlatform::hasGamepad() or could we just assume the input holder type will always be correct?
There was a problem hiding this comment.
You're right, but I think that would need to use a controller disconnect event to change the input back to default (kbm or touchscreen), if not, it would be stuck at the controller input type while you don't use any input
It's worth mentioning Options::m_bUseController is being used for locking the controller input, not sure if that's clear at all
|
|
||
| inReset = false; | ||
|
|
||
| if (x) |
There was a problem hiding this comment.
Could you manually write this out as x != 0.0f? Same below.
| int index; | ||
| switch (ctrl) | ||
| { | ||
| case KM_FORWARD: |
There was a problem hiding this comment.
I'd recommend cramming these onto one line each.
| @@ -49,6 +50,7 @@ class Font | |||
| int field_0; | |||
| int m_charWidthInt[256]; | |||
Game mode selector included, right? |
3 participants
TODO