Keine Ahnung aber ein Dos Spiel programmiert

drcubeman

Hallo, ich bringe mir C++ durch das Buch "C++ Für Spieleprogrammierer" selbst bei und finde es toll. Jedoch kann mir dieses Buch nicht sagen ob das was ich mache, richtig oder falsch ist. Deswegen bitte ich euch mir ein paar Tipps und logische Tricks zu zeigen, mich würde auch ein weiterer guter Buchtipp sehr freuen.

Mein Spiel ->

Main

#include "Game.h"

int main() 
{
	//Spieldaten laden
	CGame Game;
	Game.Init();

	//Spiel abspeieln
	Game.Interface();
		Game.Run();

	//Spiel ende
	return 0;
}

Game.cpp
http://codeviewer.org/view/code:4a5f

Game.h
http://codeviewer.org/view/code:4a5e

Exe
[url]https://mega.co.nz/#!zwYVDLyS!u0tKWQBcjACzS1TBarzaXk-hc0bptAzCouxy4vECdFQ[/url]

Du solltest keine Funktion wie Init schreiben, sondern den Konstruktor nutzen.

Working benötigt keine Parameter, da die Funktion auf Workplace ohnehin zugreifen kann.

Die Initialisierung des Zufallsgenerators gehört nicht in die Funktion Random, sondern einfach in main() und gut ist. Dann brauchst du auch kein FirstRun abfragen.

int temp, temp2, work;
temp2 = Level + 2;
temp = Level / 2;
work = Random(temp, temp2);

Warum so umständlich?

int work = Random(Level / 2, Level + 2);

if (Ham <= 0){
if (Chese <= 0){
if (Bread <= 0){
cout << "Du hast nicht mehr genug zu Essen!" << endl;
}
}
}

Schau dir mal && (Und) und || (Oder) an...

//Level Up und verbesserung der Werte
if (Level == 1 && Exp >= 40)
{	
Level++;
MaxHitpoints = MaxHitpoints + 8;
MaxManapoints = MaxManapoints + 8;
Exp = Exp - 40;
}
if (Level == 2 && Exp >= 55)
{
Exp = Exp - 55;
Level++;
MaxHitpoints = MaxHitpoints + 8;
MaxManapoints = MaxManapoints + 8;
}
if (Level == 3 && Exp >= 69)
{
Exp = Exp - 69;
Level++;
MaxHitpoints = MaxHitpoints + 8;
MaxManapoints = MaxManapoints + 8;
}

Schau dir mal Arrays an. Damit kann man sowas vereinfachen...

In C++ inkludiert man nicht stdlib.h sondern cstdlib, etc...

Es gibt sicher noch viel mehr was man anders machen kann, aber es ist doch recht viel Code und so schnell durschaue ich das auch nicht.

SeppJ

Ein eher allgemeineres Problem ist, dass du eine Gottklasse geschrieben hast.

drcubeman

Vielen dank euch beiden, es sind zwar Kleinigkeiten aber das hilft mir ungemein. Zum Thema Gottklasse, welche Aufteilung ist am sinnvollsten? Ich würde es jetzt nach ...

Game
Work
Marketplace
Dungeon
Day

... und alles, was damit zu tun hat, in diese Kategorien sortieren.

Mechanics

Mir gefallen vor allem die ganzen hartkodierten Sachen nicht:

Level == 3 && Exp >= 69

Denk doch gleich Richtung Level Editor. Du musst die Logik von den Daten und der Darstellung trennen. Überleg dir ein Format, in dem du die Level speichern kannst. Am besten irgendein einfaches Textformat oder XML. Das kannst du dann mit einem Texteditor bearbeiten. Dann hättest du im Programm eine Engine, die das ausführen kann. Die ganzen Bedingungen, Level, Items, Erfahrungspunkte usw. wären Daten und kein Programmcode mehr. Und dann könntest du das ganze immer weiter ausbauen, z.B. irgendwelche Trigger usw. hinzufügen. Das wäre auch zum Lernen wahrscheinlich interessanter.

Ich denke, ein wichtiger Hinweis für den Anfang ist es, doppelten bzw. ähnlichen Code zu vermeiden. Wenn du deine Logik oder auch nur einen beschreibenden Text (wie "Diese Arbeit bringt dir...") ändern möchtest, musst du das an zig Stellen machen. Das ist aufwändig und wahrscheinlich übersiehst du einige Stellen, oder baust versehentlich Fehler ein. Bei kleineren Projekten ist das bestenfalls lästig, aber wenn dein Projekt größere Dimension erreicht, wird es dich in den Wahnsinn treiben.
Wenn du dich also dabei ertappst, copy und paste zu benutzen, sollten in deinem Kopf Alarmglocken läuten.

Hier als Beispiel eine überarbeitete Version deiner "Working"-Methode:

int GetPaymentForWorkplace(int Workplace)
{
    if (Workplace <= 3)return 10;
    else if (Workplace >= 4 && Workplace <= 5)return 12;
    else if (Workplace >= 6 && Workplace <= 8)return 16;
    else if (Workplace >= 9 && Workplace <= 12)return 21;
    else return 30;
}

void CGame::Working(int Workplace)
{
    int Payment = GetPaymentForWorkplace(Workplace);
    Gold += Payment;
    Day();
    cout << "Diese Arbeit bringt dir " << Payment << " Gold." << endl;
}

Ein zweites Problem sind s.g. magic numbers, also Zahlen, die zwar für etwas bestimmtes stehen, aber im Quellcode trotzdem nur als nichtssagende Zahl dastehen.
Im obigen Code habe ich z.B. keine Ahnung, was ein Workplace von 8 bedeutet. Ist das ne Gehaltsklasse? Oder aber steht die 8 für die Cocktail-Bar Tutti-Frutti? Sollte letzteres der Fall sein, sollte der Code besser so aussehen:

if (Workplace == WORKPLACE_COCKTAIL_BAR_TUTTIFRUTTI)return 16;
[...]

Wobei die Konstante WORKPLACE_COCKTAIL_BAR_TUTTIFRUTTI an anderer Stelle als 8 definiert wäre.

SeppJ

drcubeman schrieb:

Vielen dank euch beiden, es sind zwar Kleinigkeiten aber das hilft mir ungemein. Zum Thema Gottklasse, welche Aufteilung ist am sinnvollsten? Ich würde es jetzt nach ...

Game
Work
Marketplace
Dungeon
Day

... und alles, was damit zu tun hat, in diese Kategorien sortieren.

Nein, eine Klasse ist keine Sammlung von Dingen und Funktionen, die irgendwas mit einer Kategorie zu tun haben. Das wäre eher ein Namespace.

Eine Klasse soll ein Objekt deiner Vorstellungskraft nachbilden. Mit genau den Eigenschaften und Fähigkeiten, die dieses Objekt auszeichnen, nicht mehr und nicht weniger. Es kann gut sein, dass dein Spiel hinterher sehr, sehr viele Klassen hat. Und es gibt auch keine Patentlösung zur Aufteilung von Klassen für Spiele. Schließlich kommt es ganz auf das Spiel an, welche Objekte eine Rolle spielen.

xaxa schrieb:

Hier als Beispiel eine überarbeitete Version deiner "Working"-Methode:

int GetPaymentForWorkplace(int Workplace)
{
    if (Workplace <= 3)return 10;
    else if (Workplace >= 4 && Workplace <= 5)return 12;
    else if (Workplace >= 6 && Workplace <= 8)return 16;
    else if (Workplace >= 9 && Workplace <= 12)return 21;
    else return 30;
}

und in C++ ist es kaum länger:

int getPayment(int n)
{
  std::map<std::pair<int, int>, int> _m;
  _m[{ 4, 5 }] = 12;
  _m[{ 6, 8 }] = 16;
  _m[{ 9, 12 }] = 21;
  for (auto i : _m)
    if (n >= i.first.first && n <= i.first.second)
      return i.second;
  return 30;
}

großbuchstaben schrieb:

und in C++ ist es kaum länger:

int getPayment(int n)
{
  std::map<std::pair<int, int>, int> _m;
  _m[{ 4, 5 }] = 12;
  _m[{ 6, 8 }] = 16;
  _m[{ 9, 12 }] = 21;
  for (auto i : _m)
    if (n >= i.first.first && n <= i.first.second)
      return i.second;
  return 30;
}

Wenn schon eine map, dann wenigstens lower_bound verwenden. Über eine map loopen ist langsam.

bei 5 entries dürfte das kaum was ausmachen, aber wenn's auf Performance ankommt, dann geht auch das hier:

#define IF(w, low, high, amount) if (w >= low && w <= high) return amount
...
IF(Workplace, 4, 5, 12);
IF(Workplace, 6, 8, 16);
IF(Workplace, 9, 12, 21);

oder auch als inline Funktion, wenn es kein Makro sein soll

Wenns auf Performance ankommt, nimmt man ein std::array und macht entweder binary_search drüber oder iteriert (je nach Anzahl Elemente). Auf keinen Fall Makros.

Aber über eine map loopen sollte man aus Prinzip nie.

großbuchstaben schrieb:

und in C++ ist es kaum länger:

Kannste jetzt aber beim IOCCC einreichen (+/-). Das Problem ist nicht der Ansatz an sich, sondern dass die (wenn hier auch sehr simple Logik) nicht hinter einer geeigneten Datenstruktur weggekapselt ist. Unter anderem störe ich mich daran, wenn ich "first" und "second" außerhalb von Bibliothekscode lese (in der vollendeten Form dann sowas wie p.first.second.first->p.second).
Ein Anfänger hat noch keine eigene Sammlung an Klassenbibliotheken geschrieben und benutzt auch keine anderen Bibliotheken wie boost, die spezialisierte Container und Datenstrukturen anbieten. Daher sollten die Bälle hier besser flach gehalten werden.