#include "stdafx.h"
#include "GameEngine.h"

GameEngine::GameEngine(void)
{
	GameState = Uninitialized;
	GameTitle = "2D SFML RPG";
	ScreenWidth = 1920;
	ScreenHeight = 1080;
	Section = 0;
	GameView.reset(sf::FloatRect(0,0,ScreenWidth,ScreenHeight));
	GameView.setViewport(sf::FloatRect(0,0,1,1));
}

void GameEngine::Start()
{
	if(GameState != Uninitialized)
		return;
	
	GameWindow.create(sf::VideoMode::getDesktopMode(), GameTitle);
	GameWindow.setFramerateLimit(30);
	GameWindow.setVerticalSyncEnabled(true);
	Running = true;
	GameState = State::Title;
	Loop();
}

void GameEngine::Loop()
{
	while(Running)
	{
		GameWindow.setView(GameView);
		GameWindow.clear();
		Event(GameState);
		switch(GameState)
		{
			case State::Title:
				Menu.Show(GameWindow);
				break;
			case State::CharacterCreation:
				CCreate.Show(GameWindow);
				MapLocation = Location::HomeTown;
				break;
			case State::Play:
				MapSelection();
				break;
		}
		GameWindow.display();
	}
}

void GameEngine::Event(State GState)
{
	while(GameWindow.pollEvent(event))
	{
		if(event.type == sf::Event::Closed)
			Running = false;
		switch(GState)
		{
			case State::Title:
				Menu.Event(GameState,Running,event,GameWindow);
				break;
			case State::CharacterCreation:
				CCreate.Event(GameState,Running,event,GameWindow,Character);
				break;
			case State::Play:
				GameMap.Event(GameState,Running,event,GameWindow,GameView);
				Character.Event(event,GameWindow,GameView,PlayerLocation);
				TestNPC.Event(event);
				break;
		}
	}
}

void GameEngine::MapSelection()
{
	switch(MapLocation)
	{
		case Location::HomeTown:
				GameMap.Show(GameWindow);
				Character.Show(GameWindow);
				TestNPC.Load(MapLocation);
				TestNPC.Show(GameWindow,PlayerLocation);
				break;
	}
}

GameEngine::~GameEngine(void)
{
}

GameEngine.h

#pragma once

#include "MainMenu.h"
#include "CharacterCreator.h"
#include "Map.h"

class GameEngine
{
private:
	sf::RenderWindow GameWindow;
	sf::Event event;
	sf::View GameView;
	int ScreenHeight;
	int ScreenWidth;
	int Section;
	char *GameTitle;
	bool Running;
	sf::Sprite PlayerLocation;
	MainMenu Menu;
	CharacterCreator CCreate;
	Map GameMap;
	Player Character;
	NPC TestNPC;
	Location MapLocation;
public:
	GameEngine(void);
	~GameEngine(void);
	void Start();
	void Loop();
	void Event(State GState);
	void MapSelection();
};

stdafx.h

#pragma once
enum Location {HomeTown, Dungeon1};
static Location MapLocation;
#include "targetver.h"

#include <stdio.h>
#include <tchar.h>
#include <string>
#include <iostream>
#include <fstream>
#include <SFML\Graphics.hpp>

#include "Player.h"
#include "NPC.h"

enum State {Uninitialized, Splash, Title, CharacterCreation, Play};
static State GameState;

static sf::Color Red = sf::Color(255,0,0);
static sf::Color Black = sf::Color(0,0,0);
static sf::Color White = sf::Color(255,255,255);

static State GameState;

static State GameState;

With this in a header file, every cpp file that includes it is going to get its own internally-linked copy of the variable named GameState. This is probably not what you want.

Why does this variable need to be in the header, anyway? I searched for it in your github, and I only saw it in GameEngine.cpp. I suppose that's why you haven't noticed any bugs yet.

Didn't notice that issue. Thanks for pointing it out. I honestly can't remember why I put it there.

You could try showing your game and asking about this on the SFML forum too (unless you did and I missed it).

I would probably make a game state class rather than doing all those switch statements in your GameEngine functions.
This could either be a polymorphic class, or a template class with an integer template argument, or a class that does these switch statements for you.
I would do a polymorphic class. vfunc overhead should be negligible as the gamestate should rarely change.

same with map location.

the more I program, the more it starts to get disorganized

No, the more you stuff things into a class the more it gets disorganized. :wink:

A tipical work around to not chase waterfalls here is to follow the SRP (Single Responsability Principle). In your case you basically would separate the game, game state, and engine in classes having unique responsabilities. A game has game states which handles the updating and rendering of the game, game states have player information that is not persistent through the entire game and implements game logic scripts, and an engine can contain low-level stuff such as a OS window in your example/catch keyboard presses and releases to be used by the game/load data resources. You compose high level modules of the game using the lower ones. Later you'll se that that is not only pottentially adds to code organization and maintence but also adds to memory performance which is a optimization topic for another discussion. Note that I'm assuming you're a beginner here for me to not describe complex solutions. As a starting point, always keep code simple and maintanable.

Also is there anything I could remove and incorporate into Lua?

Game logic scripts. But that is a topic for another discussion as well. You should hardcode them in the game and in game states in the abscence of Lua.

It wouldn’t hurt to make your code readable. Use whitespace and several other coding practices to improve readability.

Taking the same chunk of code LSpiro quoted, there's many variables there that aren't needed, and some of the names need to be cleaned up as well:

private:
	sf::RenderWindow GameWindow;
        sf::View GameView;

	//sf::Event event; //<-- This is a temp variable, so why is it persisted? Also, why is it lowercase when everything else is uppercase?
	
        //Width and heights go together so much, you really ought to have a struct that keeps them together.
        //Further, you don't even need these, because these are either made redundant by sf::RenderWindow's "getSize()" function,
        //or else made redundant by sf::View's "getSize()", depending on whether your "screen" terminology means "Window resolution" or "view resolution"
        //It's also important to try and be consistent in your terminology.
	//int ScreenHeight;
	//int ScreenWidth;

	int Section; //What is 'section'?

        //(A) Make this std::string
        //(B) you are violating const-correctness by pointing a non-const char at a string-literal
        //    (this is only permitted for backwards compatibility, due to archaic rules).
	char *GameTitle; 
	
        bool Running; //This is redundant with sf::RenderWindow's  "isOpen()"  function
        
	sf::Sprite PlayerLocation; //This is a sprite... why's it called "Location"? If all you are wanting is the player's most recent global bounds, why not just save that?
	MainMenu Menu;
	CharacterCreator CCreate; //Why's this have some weird abbreviated name? 'CCreate'!?
	Map GameMap;
        
        //This is another example where you should strive to be consistent in your terminology.
        //Why use both 'Player' and 'Character' if they have identical meanings in your code?
	Player Character;
	NPC TestNPC;
	Location MapLocation;

It looks like most of your naming is trying to jump around name collisions with the types themselves.
For example, it looks like you want names like "Map Map", "MainMenu MainMenu", and "Player Player", but you can't, so you do "Map GameMap", "MainMenu Menu", and "Player Character".

If you named your class types with an uppercase, but your instances with lowercases, you wouldn't have that problem.
It'd then be: "Map map", "MainMenu mainMenu", and "Player player"

private:
        std::string gameTitle; 
	sf::RenderWindow window;
        sf::View view;

	int section;

	MainMenu mainMenu;
	CharacterCreator characterCreator;
	GameMap gameMap;
        Location location;
   
	Player player;
	NPC testNPC;

This would be further cut down, by making NPCs and other entities part of the map they are in, and making an array of gamestates that you index into with your enum, instead of manually switch()ing in every location you need to access gamestates.

Instead of doing things like this: (saving things like 'PlayerLocation')

Character.Event(event,GameWindow,GameView,PlayerLocation); //Unnecessarily saving the player's location, now having to keep it in-sync with the actual player...

//Later...
TestNPC.Show(GameWindow,PlayerLocation); //Passing in an entire sprite when all you want is the global bounds...

You add a function to 'Player', and do this:

Character.Event(event,GameWindow,GameView);

//Later...
TestNPC.Show(GameWindow, Character.getGlobalBounds());

I have two resources to suggest.

1. Lots of useful information, like a bible for clean coding:

https://www.bbv.ch/images/bbv/pdf/downloads/Clean_Code_Cheat_Sheet_V1.2.pdf

2. You can watch these design pattern videos from Derek Banas for ideas:

https://www.youtube.com/playlist?list=PLF206E906175C7E07