advanced navmap base init - #20
Conversation
Lazrius
left a comment
Lazrius
left a comment
There was a problem hiding this comment.
Looks good, mostly just some minor stylistic changes
| std::vector<std::string> displayable_systems; | ||
|
|
||
| // Configuración del mapa | ||
| bool show_player_position = true; |
There was a problem hiding this comment.
Can we make it so our fields are camelCase to keep things consistent
| std::string name; | ||
| float posX; | ||
| float posY; | ||
| float scale = 1.0f; // escala del sistema para el navmap |
There was a problem hiding this comment.
Would it be possible make this comments in English so we can all understand them? : )
| ~NavmapWindow(); | ||
| void RenderWindowContents() override; | ||
|
|
||
| struct VisitedSystem { |
There was a problem hiding this comment.
Can you make sure that in whatever IDE you are using, clang-format is activated so the code format conforms to the standard?
| std::vector<VisitedSystem> found_systems; | ||
|
|
||
| private: | ||
| char systemNameBuf[128]; |
There was a problem hiding this comment.
Can you change this to an std::array<char, 128>
|
|
||
| class NavmapWindow final : public FlWindow | ||
| { | ||
| public: |
There was a problem hiding this comment.
Our classes are typically private, public order
| } | ||
| if (hasVisitedBase) { | ||
| auto nameW = Fluf::GetInfocardName(sys->idsName); | ||
| std::string name = WideToUtf8(nameW); |
There was a problem hiding this comment.
We have a standard function for converting between the two. We use StringUtils::stows / wstos almost everywhere else
| OutputDebugStringA("RENDER: Continuing because showMessage is TRUE\n"); | ||
|
|
||
| // Render the NavmapWindow if it exists | ||
| if (navmapWindow) { |
There was a problem hiding this comment.
is it possible for this to be null? If ImGui is not active in the OnGameLoad hook, we should throw a ModuleLoadException to unload the dll.
| @@ -0,0 +1,944 @@ | |||
| #include "PCH.hpp" | |||
| #include "../Include/advanced_navmap.hpp" | |||
There was a problem hiding this comment.
Include should be implicitly available, so no need to do the "../Include" part
| { | ||
| g_navmapWindow = this; | ||
| CenterWindow(); | ||
| SetSize(ImVec2(620, 700)); |
There was a problem hiding this comment.
In a recent update, I added SetSizeWithAspectRatio, which is a lot better to use in my opinion
| return s; | ||
| } | ||
|
|
||
| void NavmapWindow::RenderWindowContents() |
There was a problem hiding this comment.
This function is massive, it might be a good idea to split it up into smaller functions for rendering specific bits which this function then calls.
2 participants
No description provided.