This-Zeiger ist NULL

hustbaer

@frankthefox sagte in This-Zeiger ist NULL:
Bei

window_.reset(new GameWindow(args));

wird window_ erst geändert nachdem der Konstruktor von GameWindow fertig gelaufen ist. Bis dahin hat window_ noch den alten Wert, in deinem Fall halt NULL. Da der Aufruf von Game::HandleCreate noch während der Ausführung des Konstruktors von GameWindow kommt...

FrankTheFox

@martin-richter

Danke für den Hinweis, aber im Game-Konstruktor setzte ich doch static_this, im Destruktor dann wieder auf null.

FrankTheFox

@hustbaer

Ich denke, das ist es. Ich erzeuge die Fenster-Ressource im Konstruktor. Die Nachrichten WM_NCCREATE, WM_CREATE werden and die GameWindowProc gesendet. Der Konstruktor von GameWindow ist allerdings nicht komplett durchgelaufen, daher der Fehler. Ich teste das mal mit einem Default-Konstruktor und einer CreateWindow - Funktion, also so ein two-stage-construction Ansatz. Sollte das Problem aus der Welt schaffen. Allerdings finde ich das der Konstruktor die Instanz auch initialisieren sollte.

hustbaer

@frankthefox Naja das ganze ist sowieso nicht sauber gekapselt. Game ist ein Singleton und agiert als Dispatcher für die Window-Messages. Und verlässt sich darauf dass eh immer nur ein Fenster existieren kann. Von daher würde two-phase Construction das ganze nicht mehr viel schlimmer machen.

Saubere Kapselung inklusive "one phase" Construction ist bei WinAPI aber generell recht schwer hinzubekommen. Wenn man darauf verzichtet die ersten paar Messages an das jeweilige Objekt zu dispatchen (WM_NCCREATE & Co.), dann geht es indem man den Zeiger auf das C++ Objekt als "window long ptr" setzt (SetWindowLongPtr). Und in der Window-Proc dann entsprechend wieder über GetWindowLongPtr rausholt. Ein weiterer Nachteil dieser Technik ist aber natürlich dass das mit anderem Code kollidieren kann, wenn dieser ebenso (SetWindowLongPtr) verwendet.

Die "beste" Variante die ich bis jetzt kenne ist jedem Fenster eine eigene Window-Proc zu geben die dynamisch generiert wird. Dabei generiert man einen kleinen Thunk in dem der Zeiger auf das Objekt mit abgespeichert ist. Der Thunk ruft dann, mit diesem Zeiger als zusätzlichen Parameter, die "echte" Window-Proc auf. Der Nachteil dieser Variante ist wieder dass sie nicht portierbar ist: du brauchst pro Compiler und/oder CPU-Architektur die du unterstützen willst eigenen Code. Wie das aussehen kann, kannst du dir z.B. in den ATL Sourcen ansehen.

FrankTheFox

@hustbaer
Danke für die ausführliche Antwort. Ich habe bislang mich mit der SetWindowLongPtr-Variante nicht wirklich anfreunden können. Hatte Beiträge bei z.B. Stackoverflow gesehen, die Probleme sehen, weil der Bereich überschrieben werden kann und dann null-Zeiger dereferenziert werden. Die Thunk-Methode ist leider eine Methode die Compiler/CPU abhängig ist. Der Beispiel-Code ist nicht gekapselt, weil mir mal Gedanken gemacht habe wie ich das angehen konnte. Ich werde mal schauen das ganze sauber zu implementieren.

yahendrik

Du kannst auch zusätzlichen Speicher für die Fensterklasse anfordern (WNDCLASS::cbWndExtra) und die Instanz (oder eine WNDPROC) dort speichern, falls du GWLP_USERDATA (das meint Hustbär) anders verwenden möchtest.
Das ist zumindest sehr viel besser, als gleich mit einer solch krassen Einschränkung ein Projekt zu beginnen.

FrankTheFox

@yahendrik

Ich weiß das ein statischer Zeiger sowie ist eine two-phase-construction nicht das gelbe vom Ei sind. Ich könnte auch 'ne std::map nehmen und dort eben Zeiger und HWND's speichern, Da ich aber nur ein einziges Fenster brauche, habe ich diese Möglichkeit ausgeschlossen. Maps (nicht std) werden ja z. B. von der MFC benutzt. CWnd's werden dort abgelegt. CWnd::FromHandle(hWnd) usw.

FrankTheFox

Variante 1:

So würde ich es machen wenn mehrere Fenster existieren:

 class GameWindow
{
    static GameWindow* wnd_being_created;
    static std::unordered_map<HWND, GameWindow*> hwnd_map;

    HWND wnd_;


    static LRESULT CALLBACK Window::GameWindowProc(
                                               HWND wnd, 
                                               UINT msg, 
                                               WAPRAM wparam, 
                                               LPARAM lparam);

public:
    void Create (const GameArgs& args)
    {
        wnd_being_created = this;
        ::CreateWindow (args);
    }



};




static LRESULT CALLBACK GameWindow::GameWindowProc(
                                             HWND wnd, 
                                             UINT msg, 
                                             WAPRAM wparam, 
                                             LPARAM lparam)
{
   GameWindow* this_window {};

   if(wnd_being_created)
   {
       hwnd_map[HWND] = wnd_being_created;
       wnd_being_created->wnd_ = wnd;
       this_window = wnd_being_created;
       wnd_being_created = nullptr;

       return DefWindowProcW(hwnd, msg, wparam, lparam);
   }


   auto window = hwnd_map.find(wnd);
   this_window = window;

   return this_window->WindowProc(msg, wparam, lparam);
}

Variante 2:

Das ist der Tipp von euch mit GWLP_USERDATA mal von mir aus der Hüfte geschossen und kurz skizziert:

 
wc.cbWndExtra = sizeof(GameWindow *);

HWND wnd = CreateWindowExW(...);

SetWindowLongPtr(wnd, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this));


LRESULT CALLBACK GameWindow::GameWindowProc(
                                       HWND wnd,
                                       UINT msg,
                                       WPARAM wparam,
                                       LPARAM lparam)
{
     GameWindow *window = reinterpret_cast<GameWindow *>(GetWindowLongPtr(wnd,GWLP_USERDATA));

     return window->wndProc(msg, wparam, lparam);
}

So ist das doch gemeint...also jetzt diese Variante...

hustbaer

@FrankTheFox
Ja genau so war das gemeint. (Wobei ich nicht weiss ob man für GWLP_USERDATA überhaupt wc.cbWndExtra > 0 braucht - aber schaden tut's sicher nicht.)

Was bei den Maps wieder doof ist ist dass die Sache nicht thread-safe ist. Für ne Library bzw. allgemein wiederverwendbaren Code halt doof. Für ein Spiel das nur einen UI-Thread hat natürlich egal. Oder auch nicht egal. Doof ist nämlich wenn es so aussieht als ob es wiederverwendbar wäre, und dann solche Fallstricke darin begraben sind. Also vielleicht zumindest überall wo auf diese statischen Variablen zugegriffen wird ein assert(::GetCurrentThreadId() == ui_thread_id); reinmachen. Damit kann man sich erstmal unnötige Arbeit bzw. Overhead das ganze thread-safe zu machen sparen, aber die Gefahr dass jemand (vielleicht sogar man selbst) irgendwann darüber stolpert ist minimiert.

FrankTheFox

@hustbaer

Hab mal ein fast vollständig funktionierendes Beispiel...Hier setzte ich allerdings in der WindowProc den Zeiger...darum der Umstand mit dem static pointer...wenn ich also den Zeiger schon haben will wenn die WM_CREATE Nachricht aufschlägt.

#include <windows.h>
#include <string>
#include <iostream>
#include <stdexcept>



class GameWindow
{
public:
    GameWindow(HINSTANCE instance);



    bool CreateGameWindow(HINSTANCE instance);
    std::wstring GetWindowClassName(HINSTANCE instance);

private:
    static GameWindow* wnd_being_created;
    static LRESULT CALLBACK GameWindowProc(
                                        HWND wnd,
                                        UINT msg,
                                        WPARAM wparam,
                                        LPARAM lparam);

    LRESULT WindowProc(
                    UINT msg,
                    WPARAM wparam,
                    LPARAM lparam);

    HWND wnd_;

};

GameWindow* GameWindow::wnd_being_created;


GameWindow::GameWindow(HINSTANCE instance):
    wnd_ {}
{
    static const auto kWindowClassName = GetWindowClassName(instance);

    if(kWindowClassName.empty()){
        throw std::runtime_error("Failed to register window class.");
    }

    wnd_being_created = this;

    HWND wnd = CreateWindowExW(
                              0,
                              kWindowClassName.c_str(),
                              L"",
                              WS_POPUP,
                              0, 0, 32, 32,
                              nullptr,
                              nullptr,
                              instance,
                              nullptr);

    if(wnd != wnd_){
        throw std::runtime_error("Failed to create window.");
    }
}

std::wstring GameWindow::GetWindowClassName(HINSTANCE instance)
{
WNDCLASSEXW wnd_class {};

    wnd_class.cbSize = sizeof(WNDCLASSEXW);
    wnd_class.style = CS_OWNDC | CS_VREDRAW | CS_HREDRAW;
    wnd_class.hInstance = instance;
    wnd_class.cbWndExtra = sizeof(GameWindow *);
    wnd_class.lpfnWndProc = GameWindowProc;
    wnd_class.lpszClassName = L"GameWindowClass";
    wnd_class.hbrBackground = static_cast<HBRUSH>(
                                            GetStockObject(BLACK_BRUSH));
    wnd_class.hIcon = reinterpret_cast<HICON>(
                                    LoadImage(
                                            nullptr,
                                            IDI_APPLICATION,
                                            IMAGE_ICON,
                                            0,0,
                                            LR_DEFAULTSIZE | LR_SHARED));
    wnd_class.hCursor = reinterpret_cast<HCURSOR>(
                                    LoadImage(
                                            nullptr,
                                            IDC_ARROW,
                                            IMAGE_CURSOR,
                                            0,0,
                                            LR_DEFAULTSIZE | LR_SHARED));
    if(!RegisterClassExW(&wnd_class)){
        return L"";
    }

    return wnd_class.lpszClassName;
}

LRESULT CALLBACK GameWindow::GameWindowProc(
                                        HWND wnd,
                                        UINT msg,
                                        WPARAM wparam,
                                        LPARAM lparam)
{
    GameWindow *window = reinterpret_cast<GameWindow *>(
                                GetWindowLongPtr(wnd, GWLP_USERDATA));

    if(!window)
    {
        SetWindowLongPtr(
                        wnd,
                        GWLP_USERDATA,
                        reinterpret_cast<LONG_PTR>(wnd_being_created));
        wnd_being_created->wnd_ = wnd;
        wnd_being_created = nullptr;

        return DefWindowProcW(wnd, msg, wparam, lparam);
    }



    return window->WindowProc(msg, wparam, lparam);
}

LRESULT GameWindow::WindowProc(
                            UINT msg,
                            WPARAM wparam,
                            LPARAM lparam)
{
    std::cout << "message:  "
              << msg << "  wparam:  "
              << wparam << "  lparam:  "
              << lparam << std::endl;

    return DefWindowProcW(wnd_, msg, wparam, lparam);
}




int WINAPI WinMain(
                HINSTANCE instance,
                HINSTANCE,
                LPSTR,
                int)
{
    GameWindow window {instance};

    return EXIT_SUCCESS;
}

Mit diesem Beispiel kann ich mich auch anfreunden

Ausgabe:

message: 131 .... WM_NCCALCSIZE
message: 1 .... WM_CREATE
....

Process returned 0 (0x0) execution time : 0.017 s
Press any key to continue.

hustbaer

Wenn du wnd_being_created jetzt noch thread local machst, dann wäre das schonmal nicht schlecht.

FrankTheFox

Zuerst habe ich mich gefragt ob es einen Unterschied macht, ob eine statische Variable thread_local ist oder nicht. Aber der C++ Standard sagt dazu (n4296.pdf)

3.7.2 Thread storage duration [basic.stc.thread]
1 All variables declared with the thread_local keyword have thread storage duration. The storage for these entities shall last for the duration of the thread in which they are created. There is a distinct object or reference per thread, and use of the declared name refers to the entity associated with the current thread.
2 A variable with thread storage duration shall be initialized before its ﬁrst odr-use (3.2) and, if constructed, shall be destroyed on thread exit.

Die akzeptierte Antwort hier Are C++11 thread_local variables automatically static? sagen das thread_local auch bei statischen Variablen durchaus Sinn macht.
Vielen Dank an alle für die hilfreichen Tipps und Antworten