プログラマ専用SNS ミクプラ

はじめまして、こーもりと申します、ついでに学生です。
皆さんの手を借りたくて書かせていただきます。

早速ですが僕はBCC developerとDXLibでC++のゲームを作っています。

クラスで作っているのですが、変なバグにあってしまい開発がうまく進まない状況です。
const *charでステージの形を受け取り、もともと持っているint *Data形の中に変換して入れるというものなんですが、
*Dataの中に入れたものを画面に出してみたら、最初の二つだけが突拍子もない数字になってしまいます。

ステージの形というのは
## # ## ## <-#はブロック
########## <- は何もないところ
のようなものです。

なんででしょう(泣)……とりあえずその変換のコードは以下の通りです。

コード:

void StageClass::CreateStage(const char *StageChar,int inWidth,int inHeight,int inChipSize){

	width = inWidth;
	height = inHeight;
	chipSize= inChipSize;
	Data = new int[(inWidth*inHeight)];
	
	const char *d = StageChar;
	int i=0;
	while(*d != '\0'){
		int temp = STAGE_EMPTY;
		switch( *d ){
		case '#': temp = STAGE_BLOCK;	break;
		case ' ': temp = STAGE_EMPTY;	break;
		default : temp = STAGE_EMPTY;	break;
		}
		++d;
		Data[i] = temp;
		i++;
	}
}

STAGE_EMPTY,STAGE_BLOCKはenumで列挙しているものです。

どこが悪いのか、自分で考えても分からず、友達に聞いてみても分からずで困っています。
もしかしたらここではない箇所かもしれませんので指摘していただければその箇所を書いていきます。
どうしたらいいのか分からないので、皆様のアドバイスをお待ちしています。

どうかよろしくお願いします。

ソースが足りないですね。

CreateStageを呼ぶところと、Dataを使って描画しているところを見せてください。
ってーかStageClassクラス全部かな。

（一番手っ取り早いのは、プロジェクト全体のソースをzipにして添付することです。差支え無ければですけどね）

へにっくすさん、返信ありがとうございます！

えっと、コードはこんな感じです。
Stage.cpp

コード:

#include "stage.h"
#include "DxLib.h"


StageClass::StageClass(){
}

void StageClass::CreateStage(const char *StageChar,int inWidth,int inHeight,int inChipSize){

	width = inWidth;
	height = inHeight;
	chipSize= inChipSize;
	Data = new int[(inWidth*inHeight)];
	
	const char *d = StageChar;
	int i=0;
	while(*d != '\0'){
		StageObject temp = STAGE_EMPTY;
		switch( *d ){
		case '#': temp = STAGE_BLOCK;	break;
		case ' ': temp = STAGE_EMPTY;	break;
		default : temp = STAGE_EMPTY;	break;
		}
		++d;
		Data[i] = temp;
		i++;
	}
}

StageClass::~StageClass(){
	delete[] Data;
}

void StageClass::Draw(SymbolClass Players[],CommonClass Common){
	for(int i=0; i<width; i++){
		for(int j=0; j<height; j++){
			DrawFormatString(30+80*i,30+22*j,0xffffff,"%d",Data[i+j*width]);
		}
	}
}

呼んでいるところはこんな感じです。
別のクラスのコンストラクタでステージクラスを作って、その後CreateStageを呼んでいます。

コード:

ActionClass::ActionClass(){
	Stage = StageClass();
	Stage.CreateStage("#### #    "
					  "#  #    ##"
					  " #     ###"
					  "  #   ####"
					  "    ## ###"
					  "         #"
					  " ##     # "
					  "      ##  "
					  "   ## #   "
					  "##      ##",10,10,100);
}

とりあえずコンソールアプリケーションで検証コード組んでみたけど、特に問題なさそうですね。

コード:

// ConsoleApplication1.cpp : コンソール アプリケーションのエントリ ポイントを定義します。
//

#include "stdafx.h"

int *Data;
typedef enum _stageObject {
	STAGE_EMPTY = 0,
	STAGE_BLOCK,
} StageObject;


void CreateStage(char *StageChar, int inWidth, int inHeight, int inChipSize)
{
	//width = inWidth;
	//height = inHeight;
	//chipSize = inChipSize;
	Data = new int[(inWidth*inHeight)];

	const char *d = StageChar;
	int i = 0;
	while (*d != '\0'){
		StageObject temp = STAGE_EMPTY;
		switch (*d){
		case '#': temp = STAGE_BLOCK;   break;
		case ' ': temp = STAGE_EMPTY;   break;
		default: temp = STAGE_EMPTY;   break;
		}
		++d;
		Data[i] = temp;
		i++;
	}
}

void Draw(int width, int height){
	for (int i = 0; i<width; i++){
		for (int j = 0; j<height; j++){
			// DrawFormatString(30+80*i,30+22*j,0xffffff,"%d",Data[i+j*width]);
			printf("%d", Data[i + j*width]);
		}
		printf("\n");
	}
}

int _tmain(int argc, _TCHAR* argv[])
{
	CreateStage(
		"#### #    "
		"#  #    ##"
		" #     ###"
		"  #   ####"
		"    ## ###"
		"         #"
		" ##     # "
		"      ##  "
		"   ## #   "
		"##      ##", 10, 10, 100);
	Draw(10, 10);

	delete[] Data;
	return 0;
}

実行結果

コード:

ただしコンパイルしたのはVisual Studio 2013 Express for Windows Desktopですが。
上記のような実行結果にはなってないってことなんですよね？

はい、その通りの実行結果にならないんですよ……。

こちらでコンパイルし、実行した結果は

6760304 6760304 1 1 0 1 ...
1 0 0 1 ...
.
.
.

ってなっちゃってるんです。
(実行結果は、画像がup出来れば分かりやすいんですけどねー。)

こーもりさんが書きました:(実行結果は、画像がup出来れば分かりやすいんですけどねー。)

いや、%dで出力してるんだからそれで十分。

では、newして確保したメモリをDataに入れてから、Drawで使われるまでに、Dataポインタが書き換わっていないか、または、Dataの中身を書き換えていないかをチェックする必要がありますね。
printfの書式で%pを使うと、ポインタアドレスを16進数表示できます。
newした結果と、Drawを呼ぶ直前で、

コード:

printf("%p", Data); // またはDrawFormatStringで。

の一行を挿入したらどうなりますか？
同じなら、Dataのポインタが書き換わっていないので問題ないということになります。どこかでDataの中身が書き換わっているのです。

オフトピック

もう一度言いますが手っ取り早いのは、ソースをzipに固めて、どこかのアップローダにあげてリンクすることです。そうすれば全体を見てもらえますよ

ポインタの位置は変わっていないようです。
あと、コンパイルしたら出力される数値が変わりました！！

……？

と、とりあえず、へにっくすさんの言う通りzipにしてみました。
至らない点もあるかもしれませんし、分かりづらいかもしれませんがよろしくお願いします。

http://www1.axfc.net/u/3230878

すみません、よく考えたら「意味ないじゃん。」ってやり方してました。

他の方法でやってみたら、ポインタの位置が変わってるように見えました。
ただ、いつ変わっているのかが分かりません。

コード:

ActionClass::ActionClass(){
    Stage = StageClass();
    Stage.CreateStage("#### #    "
                      "#  #    ##"
                      " #     ###"
                      "  #   ####"
                      "    ## ###"
                      "         #"
                      " ##     # "
                      "      ##  "
                      "   ## #   "
                      "##      ##",10,10,100);
}

上記2行目には問題があります。

StageClass()で一時オブジェクトが生成された後、全メンバーがStageにコピーされます。
そのため、一時オブジェクトのデストラクタが呼ばれたあと、メンバー変数のデストラクタが呼ばれると、
同じアドレスに対してdeleteすることになり、未定義動作となります。

StageClass以外のクラスでも同様の問題があるため、今回のような現象が発生していると考えられます。

とりあえずの対策として、全ソース中から以下のような代入文を削除してみてください。

コード:

	Stage = StageClass();
	Common = CommonClass();
	Action = ActionClass();

クラス内で動的確保を行う際の注意点を、「コピーコンストラクタ　ポインタ」などのキーワードで調べてみてください。

すでにa5uaさんが原因と、暫定の対策を述べられているので、補足を述べます。

ActionClassで、以下のように宣言されていますが、

コード:

class ActionClass{
private:
	SymbolClass Players[2];
	StageClass Stage;
public:
	ActionClass();
	void Loop(GamePad Pad,CommonClass Common);
	void Draw(CommonClass Common);
};

このとき、Players[2]、Stageはいつ作成されていると思いますか？
答えはActionClassのコンストラクタが呼ばれたときにすでに作成されているのです(SymbolClass、StageClassのコンストラクタが呼ばれている)。
なのでActionClassのコンストラクタで、わざわざ

コード:

Stage = StageClass();

と書く必要はないのです。
そうでなく、たとえばStageのインスタンス生成を制御したいのであれば、

コード:

StageClass *Stage; // ポインタにする

というようにポインタにすれば、newを使って、インスタンスの生成を制御できます(つまりコンストラクタを呼ぶタイミングを自分で指定できる)。
もちろんnewしたらdeleteするのも忘れずに。でないとメモリリークしますから。
action.cppは、以下のような感じになるかな。

コード:

#include "action.h"

ActionClass::ActionClass(){
	Players[0] = SymbolClass();
	Stage = new StageClass(); // インスタンスの生成
	Stage->CreateStage("#### #    " // ポインタになったので、.を->に変える
					  "#  #    ##"
					  " #     ###"
					  "  #   ####"
					  "    ## ###"
					  "         #"
					  " ##     # "
					  "      ##  "
					  "   ## #   "
					  "##      ##",10,10,100);
}



void ActionClass::Loop(GamePad Pad,CommonClass Common){
	Players[0].Action(Pad);
	Stage->Draw(Players, Common); // ポインタになったので、.を->に変える
}

void ActionClass::Draw(CommonClass Common){
	Stage->Draw(Players, Common); // ポインタになったので、.を->に変える
}

// Stageがポインタになったのでnewしたらdelete(解放)が必要（もちろんAction.hにも宣言を忘れずに）
ActionClass::~ActionClass() {
	delete Stage; // 解放
}

ポインタの概念を勉強した方がよいですよ。

へにっくすさん、a5uaさん、返信ありがとうございました。

a5uaさんへ
暫定の対策、ありがとうございます！
やってみたら動きました！
それで、分からないことがあるので下で質問させていただきます。

へにっくすさんへ
分かりやすく解説ありがとうございます！
メモリについては苦手分野で、前に学校祭で出したプログラムも最初メモリリークしてしまっていました。(言い訳ですね、ごめんなさい。)
なるほど、ActionClassで呼んだときには、もう作成されていたんですね、知りませんでした。
しかもポインタにすればインスタンスの作るタイミングも選べるのか……ポインタにして作ってみようかな？

お二方、本当にありがとうございます！

それで今回のことで調べて、自分なりに考えてみたんですけどこういうことですか？

コンストラクタ内でポインタのメモリをあけ、変数に初期値を代入する。
それをコピーする。
デストラクタでメモリを返す。
すると、メモリが返されてしまっているので、コピーされたほうのアドレスの参照位置は何が起こっているかわからない。

ってことですかね？

えっと、でもそれだと、CreateStage内でもう一度newされたときにちゃんと使えるアドレスが入るんじゃないのかな？
って思うんですけど、どうなんでしょうか。
勉強不足ですが、教えていただけると幸いです。

こーもりさんが書きました:えっと、でもそれだと、CreateStage内でもう一度newされたときにちゃんと使えるアドレスが入るんじゃないのかな？

問題はそこではありません。
元のソースで、必要ない次の一行
Stage = StageClass();
がありましたよね。これはその関数内で、コンストラクタで作成したStageとは別の、ローカルなStageクラスを作成しているのだと思ってください。
このおかげでActionClassのコンストラクタを抜けた時にそのローカルなStageクラスのデストラクタが呼ばれてDataが解放されていたのです。
なのでDraw関数が呼ばれたときは、解放されているDataを参照していることになっていたのです。
その一行を削除したら動いたというのは、ローカルで作成していたクラスがなくなったのでデストラクタが呼ばれなくなったからです。
なのでDraw関数が呼ばれても、Dataは解放されていないため、ちゃんと動いたのです。

a5uaさんはとりあえず削除してみてください、とありましたが、
個人的にはとりあえずじゃないじゃんとか思ったりしてますｗ。

元のzipのソースをそのままVisual Studio 2013 Express for Windows Desktopに取り込んで実行してみたら落ちましたよ。
BCCでは落ちないのね…

StageClass()をしてしまったために、ActionClassの終わりにStageClassのデストラクタが呼ばれてしまってDataを開放されてしまった。
でもDrawは"開放したけどラッキーなことに変わっていなかったData"(今回でいうと、ラッキーじゃなかったんですけどね)を見て、表示させてしまっていた。

ということですか……。

それとBCCでは落ちませんでしたね、どちらにしろ自分の力だけでは解けませんでしたけど……。

それはおいておきまして、お二方とも優しくご指導いただき、ありがとうございました！
解決のための方法(やり方っていうのかな？)などを見て、まだまだ勉強不足だなと感じられました。

今回の問題と解決策
問題：配列の中身がおかしくなっている。
理由：メモリが開放されていたのに、中身を見てしまっていた。
解決方法:変なタイミングでデストラクタが呼ばれていたのを消去。

これにて解決とさせていただきます。
本当にありがとうございました！！

プログラマ専用SNS ミクプラ

C++で配列のバグ(?)が消えません

C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません

Re: C++で配列のバグ(?)が消えません