Switch returns c**p WTF????

[MrSeanKon]

This is a C# code (a part) of my cardgame:


using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Text;
using System.Windows.Forms;
using System.Threading;
using System.Diagnostics;
using System.IO;

namespace Jack
{
public partial class Main:Form
{
#region Initialization routines
public Main()
{
InitializeComponent();
for (int i=1; i<=52; i++)
combo_Choose_number.Items.Add(i.ToString());
for (int i=1; i<=20; i++)
combo_Speed_step.Items.Add(i.ToString());
}
private void Main_Load(object sender,EventArgs e)
{
if (File.Exists("UPS.ini"))
{
Read_all_settings("UPS.ini");
First_round();
}
else
{
Rectangle monitor=Screen.PrimaryScreen.Bounds;
if (monitor.Width<1280 || monitor.Height<1024)
MessageBox.Show("at 1280*1024 pixels.","The game is appeared best",MessageBoxButtons.OK,MessageBoxIcon.Information);
Menu_Actions.Visible=Menu_Other.Visible=Menu_save_ game.Visible=false;
Menu_game.Visible=true;
ptr_Move_card.Location=ptr_PC_gain_cards.Location= ptr_Player_gain_cards.Location=new Point(1100,1100);
Sto_diavolo_cards_LEET();
this.Width=580;
this.Height=375;
this.Location=new Point((int) ((monitor.Width-this.Width)/2),(int) ((monitor.Height-this.Height)/2));
ptr_Jack_photo.Location=new Point(5,35);
ptr_Jack_photo.SizeMode=PictureBoxSizeMode.AutoSiz e;
group_Options.Location=new Point(255,69);
}
}
private void Init()
{
for (int i=0; i<=52; i++)
deck[i]=decks_have_passed[i]=stacked_cards[i]=0;
for (int i=0; i<=6; i++)
PC_cards[i]=player_cards[i]=0;
PC_has_played_times=player_has_played_times=total_ passed_cards=0;
PC_grabbed_cards=player_grabbed_cards=PC_goals=pla yer_goals=0;
ptr_Jack_photo.Visible=group_Options.Visible=false ;
this.Width=935;
this.Height=830;
if (!File.Exists("UPS.ini"))
{
Rectangle monitor=Screen.PrimaryScreen.Bounds;
this.Location=new Point((int) ((monitor.Width-this.Width)/2),(int) ((monitor.Height-this.Height)/2));
}
Shuffle_the_cards();
ptr_Stack_of_cards.Visible=panel_Board.Visible=tru e;
Menu_Actions.Visible=Menu_Other.Visible=Menu_save_ game.Visible=true;
Is_PC_the_last_grabber=false;
ptr_PC_card_1.Image=ptr_PC_card_2.Image=ptr_PC_car d_3.Image=global::Jack.Properties.Resources.Card_b ack_side;
ptr_PC_card_4.Image=ptr_PC_card_5.Image=ptr_PC_car d_6.Image=global::Jack.Properties.Resources.Card_b ack_side;
ptr_PC_gain_cards.Location=ptr_Player_gain_cards.L ocation=new Point(1100,1100);
Sto_diavolo_cards_LEET();
lbl_Stack_counter.Visible=true;
if (Menu_show_progress_bars.Checked)
progress_PC.Visible=progress_Player.Visible=true;
else
progress_PC.Visible=progress_Player.Visible=false;
progress_PC.Maximum=progress_Player.Maximum=target ;
progress_PC.Value=PC_score;
progress_Player.Value=player_score;
}
private void Sto_diavolo_cards_LEET()
{ // The routine moves the cards outside of form so they are not visible in the beggining
}
#endregion

#region External variables
bool PC_first_player=false;
bool Is_PC_the_last_grabber=false;
bool animation=true;
int[] deck=new int[53];
int[] decks_have_passed=new int[53];
int[] PC_cards=new int[7];
int[] player_cards=new int[7];
int[] stacked_cards=new int[53];
int PC_score=0,player_score=0;
int PC_grabbed_cards=0,player_grabbed_cards=0;
int PC_goals=0,player_goals=0;
int PC_has_played_times=0,player_has_played_times=0;
int speed_step=6;
int total_passed_cards=0;
int target=151;
int round=1;
int total_rounds=1;
int fatses=1;
string language="English";
#endregion

#region Shuffle the cards
void Shuffle_the_cards()
{
Random shuffle;
int card;

up:
card=0;
shuffle=new Random();
for (int i=1; i<=52; i++)
{
back:
card=shuffle.Next(1,53);
for (int j=1; j<=i-1; j++)
if (deck[j] == card)
goto back;
deck[i]=card;
}
for (int i=1; i<=52; i++)
for (int j=1; j<=52; j++)
if (i != j)
if (deck[i] == deck[j])
goto up;
for (int i=1; i<=52; i++)
if (deck[i]>52 || deck[i]<1)
goto up;
}
#endregion

#region Settings
void Choose_the_first_player()
{
int player_choice=26;
// Read some values using try/catch blocks (no threads call)
int PC_choice=player_choice;
Shuffle_the_cards();
Random shuffle=new Random();
while (PC_choice == player_choice)
PC_choice=shuffle.Next(1,53);
if (deck[player_choice] > deck[PC_choice])
PC_first_player=false;
else
PC_first_player=true;
group_Options.Visible=false;
First_round();
below: int dummy=0;
dummy++;
}
private void btn_OK_settings_Click(object sender,EventArgs e)
{
Choose_the_first_player();
ptr_Move_card.BackColor=this.BackColor=Color.FromA rgb(colorDialog.Color.R,colorDialog.Color.G,colorD ialog.Color.B);
}
private void btn_Color_Click(object sender,EventArgs e)
{
colorDialog.ShowDialog();
}
#endregion

#region Sort PC cards
void Sort_PC_cards()
{
// Just sort the PC cards (no threads calling)
}
#endregion

#region Deal the cards

#region Player
void Deal_player_cards()
{
// Deal the player cards actually uses loops to set the coordinates of ptr_Move_card
if (round == 1)
{
if (PC_first_player)
{

Thread Deal_table=new Thread(new ThreadStart(Deal_table_cards));
Deal_table.Start();
}
else
{
Thread Deal_PC=new Thread(new ThreadStart(Deal_PC_cards));
Deal_PC.Start();
}
}
else
{
if (round == 2)
{
if (!PC_first_player)
{
Thread Deal_PC=new Thread(new ThreadStart(Deal_PC_cards));
Deal_PC.Start();
}
else
{
PC_play();
this.Enabled=true;
}
}
else if (round == 3)
{
if (!PC_first_player)
{
Thread Deal_PC=new Thread(new ThreadStart(Deal_PC_cards));
Deal_PC.Start();
}
else
{
PC_play();
this.Enabled=true;
}
}
else
{
if (!PC_first_player)
{
Thread Deal_PC=new Thread(new ThreadStart(Deal_PC_cards));
Deal_PC.Start();
}
else
{
ptr_Stack_of_cards.Location=ptr_Fatsa_1.Location=p tr_Fatsa_2.Location=new Point(1100,1100);
PC_play();
this.Enabled=true;
}
}
}
}
#endregion

#region PC
void Deal_PC_cards()
{
// Similar code
if (round == 1)
{
if (PC_first_player)
{
Thread Deal_player=new Thread(new ThreadStart(Deal_player_cards));
Deal_player.Start();
}
else
{
Thread Deal_table=new Thread(new ThreadStart(Deal_table_cards));
Deal_table.Start();
}
}
else
{
if (round == 2)
{
if (PC_first_player)
{
Thread Deal_player=new Thread(new ThreadStart(Deal_player_cards));
Deal_player.Start();
}
else
this.Enabled=true;
}
else if (round == 3)
{
if (PC_first_player)
{
Thread Deal_player=new Thread(new ThreadStart(Deal_player_cards));
Deal_player.Start();
}
else
this.Enabled=true;
}
else
{
if (PC_first_player)
{
Thread Deal_player=new Thread(new ThreadStart(Deal_player_cards));
Deal_player.Start();
}
else
{
ptr_Stack_of_cards.Location=ptr_Fatsa_1.Location=p tr_Fatsa_2.Location=new Point(1100,1100);
this.Enabled=true;
}
}
}
}
#endregion

#region Table
void Deal_table_cards()
{
// Similar code
if (PC_first_player && round==1)
{
MessageBox.Show("PC plays first!","Ready!",MessageBoxButtons.OK,MessageBoxIcon.Information);
PC_play();
}
else
MessageBox.Show("You play first!","Ready!",MessageBoxButtons.OK,MessageBoxIcon.Information);
}
#endregion

#endregion

#region Se visible some of the cards
void Set_visible_all_deal_cards_outside_of_baize()
{
// Move the cards only
}
void Set_visible_PC_player_deal_cards_outside_of_baize( )
{
// Move the cards only
}
#endregion

#region Rounds

#region First round
void First_round()
{
if (PC_score>=target && PC_score>player_score)
{
ptr_PC_gain_cards.Location=ptr_Player_gain_cards.L ocation=new Point(1100,1100);
Sto_diavolo_cards_LEET();
lbl_Stack_counter.Visible=progress_PC.Visible=prog ress_Player.Visible=false;
MessageBox.Show("PC is the winner!",":(",MessageBoxButtons.OK,MessageBoxIcon.Error);
File.Delete("UPS.ini");
goto Telos;
}
else if (player_score>=target && player_score>PC_score)
{
ptr_PC_gain_cards.Location=ptr_Player_gain_cards.L ocation=new Point(1100,1100);
Sto_diavolo_cards_LEET();
lbl_Stack_counter.Visible=progress_PC.Visible=prog ress_Player.Visible=false;
MessageBox.Show("You are the winner!","Congrats!",MessageBoxButtons.OK,MessageBoxIcon.Warning);
File.Delete("UPS.ini");
goto Telos;
}
lbl_PC_score.Text="Score: "+PC_score.ToString();
lbl_PC_cards.Text="Cards: 0";
lbl_Player_score.Text="Score: "+player_score.ToString();
lbl_Player_cards.Text="Cards: 0";
lbl_Stack_counter.Text="Cards: 4";
lbl_Round.Text="Round 1/"+total_rounds.ToString();
Init();
if (PC_first_player)
{
PC_cards[1]=deck[1];
PC_cards[2]=deck[2];
PC_cards[3]=deck[3];
PC_cards[4]=deck[4];
PC_cards[5]=deck[5];
PC_cards[6]=deck[6];
player_cards[1]=deck[7];
player_cards[2]=deck[8];
player_cards[3]=deck[9];
player_cards[4]=deck[10];
player_cards[5]=deck[11];
player_cards[6]=deck[12];
stacked_cards[1]=deck[13];
stacked_cards[2]=deck[14];
stacked_cards[3]=deck[15];
stacked_cards[4]=deck[16];
ptr_Stack_of_cards.Location=new Point(750,560);
panel_Board.Location=new Point(766,60);
ptr_Human_card_1.Enabled=ptr_Human_card_2.Enabled= ptr_Human_card_3.Enabled=false;
ptr_Human_card_4.Enabled=ptr_Human_card_5.Enabled= ptr_Human_card_6.Enabled=false;
}
else
{
player_cards[1]=deck[1];
player_cards[2]=deck[2];
player_cards[3]=deck[3];
player_cards[4]=deck[4];
player_cards[5]=deck[5];
player_cards[6]=deck[6];
PC_cards[1]=deck[7];
PC_cards[2]=deck[8];
PC_cards[3]=deck[9];
PC_cards[4]=deck[10];
PC_cards[5]=deck[11];
PC_cards[6]=deck[12];
stacked_cards[1]=deck[13];
stacked_cards[2]=deck[14];
stacked_cards[3]=deck[15];
stacked_cards[4]=deck[16];
ptr_Stack_of_cards.Location=new Point(750,60);
panel_Board.Location=new Point(766,560);
ptr_Human_card_1.Enabled=ptr_Human_card_2.Enabled= ptr_Human_card_3.Enabled=true;
ptr_Human_card_4.Enabled=ptr_Human_card_5.Enabled= ptr_Human_card_6.Enabled=true;
}
Sort_PC_cards();
if (stacked_cards[4]==11 || stacked_cards[4]==24 || stacked_cards[4]==37 || stacked_cards[4]==50)
Init();
else
{
// Code which shows the last deck cards
}
decks_have_passed[1]=stacked_cards[1];
decks_have_passed[2]=stacked_cards[2];
decks_have_passed[3]=stacked_cards[3];
decks_have_passed[4]=stacked_cards[4];
total_passed_cards=4;
ptr_Stack_of_cards.Visible=true;
ptr_Human_card_1.Image=Set_image(player_cards[1]);
ptr_Human_card_2.Image=Set_image(player_cards[2]);
ptr_Human_card_3.Image=Set_image(player_cards[3]);
ptr_Human_card_4.Image=Set_image(player_cards[4]);
ptr_Human_card_5.Image=Set_image(player_cards[5]);
ptr_Human_card_6.Image=Set_image(player_cards[6]);
ptr_Table_card_1.Image=Set_image(stacked_cards[1]);
ptr_Table_card_2.Image=Set_image(stacked_cards[2]);
ptr_Table_card_3.Image=Set_image(stacked_cards[3]);
ptr_Table_card_4.Image=Set_image(stacked_cards[4]);
ptr_PC_card_1.Image=ptr_PC_card_2.Image=ptr_PC_car d_3.Image=ptr_PC_card_4.Image=ptr_PC_card_5.Image= ptr_PC_card_6.Image=global::Jack.Properties.Resour ces.Card_back_side;
Set_visible_all_deal_cards_outside_of_baize();
this.Enabled=false;
if (PC_first_player)
{
Thread Deal_PC=new Thread(new ThreadStart(Deal_PC_cards));
Deal_PC.Start();
}
else
{
Thread Deal_player=new Thread(new ThreadStart(Deal_player_cards));
Deal_player.Start();
}
Telos: int dummy=0;
dummy++;
}
#endregion
}
}


The problem :mad: is that sometimes the huge switch (in Set_image routine) returns the back side of cards.
To be more specific the passing parameter (card) in the routine has the value 0.
Therefore any of cases in huge switch won't be executed.
GKR :mad: Set_image routine is called only by First_round(), Second_round(), Third_round and Fourth_round().
WTF?????? :mad:
I removed some (non necessary parts of the code) cos I must focus you the basic parts and of course a smaller size post is better.

[Kreij]

Since the call to Set_Image is bing passed a player card value, and that is earlier derived from the deck array which is set in Init() by the Shuffle_the _Cards() function, I would look there (in Shuffle_the_cards())

It is really hard to track what is happening because "some people" use goto in their code :p :D

[Kreij]

Hi MrSean,

Your shuffle card routine is a little confusing.
Does the code basically resolve like this ....


void Shuffle_the_cards()
{
Random shuffle;
int card;

up:
card=0;
shuffle=new Random();

######### This part randomly places cards in the deck
for (int i=1; i<=52; i++)
{
back:
card=shuffle.Next(1,53);
for (int j=1; j<=i-1; j++)
if (deck[j] == card)
goto back;
deck[i]=card;
}
###########

########### This part checks to make sure that no two cards are the same
for (int i=1; i<=52; i++)
for (int j=1; j<=52; j++)
if (i != j)
if (deck[i] == deck[j])
goto up;
############

############ This part makes sure no cards are out of bounds
for (int i=1; i<=52; i++)
if (deck[i]>52 || deck[i]<1)
goto up;
}


Wouldn't it be a lot easier to write the code so you didn't have to do any checking after the cards are shuffled ?

For instance ....



int tempCard;
int [] TheWholeDeck = new int [52]; // Array to hold the numbers 1 to 52 (all cards)
Random Shuffle = new Random();
Boolean GotOne = false; // Is a valid card found?

// Set the array so that it contains the value of every card
for (int everyPossibleCard = 0; everyPossibleCard < 52; everyPossibleCard++)
TheWholeDeck[everyPossibleCard] = everyPossibleCardCard + 1;

Shuffle = new Random ();

for (int EachCard = 0; EachCard < 52; EachCard++)
{
while (!GotOne)
{
tempCard = Shuffle.Next(1,52);
if (TheWholeDeck[tempCard] != 0)
{
deck[EachCard] = tempCard;
TheWholeDeck[tempCard] = 0;
GotOne = true;
}
}
GotOne = false;
}


Just a thought :)

[MrSeanKon]

I followed your instructions and here is the result:

11344

I think the game is OK now but I cannot understand why gotos were the problem.
If they were......:rolleyes:

[Kreij]

I do not think that the gotos were the problem, I believe that you had some minor error in the programming logic. That is the problem with using gotos, it makes it much more difficult to follow your code path when trying to troubleshoot.

One other thing that I would recommend is using variable names that describe what they are holding more accurately.

There is nothing inherently wrong with using something like;
"for (int i=0; i < j; i++)",
it is much easier tell what something is doing if you use more descriptive names.

Another thing you may with to avoid is using "<=" in your for loops.
If you want to loop between 0 and 100 (for example), it is much easier to use;
for (int MyVar = 0; MyVar < 101; MyVar++) {}
and the compiled code will be more compact as it will never have to check for an equality condition, only a less than.

You can see this situation if you write it out in psuedo-code.
Using <= you get ....

Is MyVar less than 100?
Yes
Loop Again
No
Is MyVar = 100?
Yes
Loop Again
No
Quit


Using just < you get ....

Is MyVar less than 101?
Yes
Loop Again
No
Quit

[MrSeanKon]

Thanks again for your tips man.
Honestly I was wondering if gotos were the problem that's why I agree with you.
But the source code you posted above included in my cardgames.

[Kreij]

Hi again MrSean,

I was looking at your code again (because I got tired of looking at mine :D ) and I noticed something odd.

In your First_round() routine, you check to see if either the player or the PC has won and then you jump to Telos:, create a dummy variable and increment it, then exit the routine.
Why?

Why don't you remove these two statements;

Telos: int dummy=0;
dummy++;


and then replace the two "goto Telos;" statements with "return;"

Just a suggestion :)
I can't see any reason you would need to increment a dummy variable.

[bruins004]

As a developer myself may I make a suggestion.
I think you should comment and seperate different lines of code a bit.
Trust me it helps a hell of a lot.

Also, use Try...Catch blocks.
That will def. help you find any errors easily and waste a lot less time.

I know they dont teach you these common principals in HS or College, but try to get ahead of the game.

Oh yea and use the Debug tool too.
It is the best thing in the world.
Setup some break points and you will see exactly where the code is bombing out.

Its always good to find the problem out on your own :)

[Kreij]

I agree with Bruins ... to a point ;)

I agree that try / catch blocks are a wonderful tool for letting your code find it's own problems. The downside is that you may have to write rather elaborate catch routines in order to output error messages that the user of your code can actually understand. Many of the system generated errors are way over the head of the average user and you will need to create your own responses to a give problem. I have seen apps that simply throw up the system generated response and 99% of users will get that "deer in the headlights" look in about 1 second.

I am not so keen on the idea of lots of comments in the code.

The reason I say this is not that comments are unneeded, but I think that if you a programmer uses descriptive variable names and seperated code blocks in routines (simple blank lines will do) that there is no need for verbose commentations unless the code is particularly complex or they are using obfuscation to help prevent disassebly or decompiling.

I have plowed through reams of code and would much rather see good technique than comments that rival a short story.

Yes, debuggers are great. Breakpoints and stepping give you all the information you will ever need to troubleshoot a problem. Especially if you are throwing things on the stack or heap and want to see exactly what the entire environment looks like at any moment.

You can also put simple statements in your code that will tell you if something is wrong by outputting it to a command console or dialog box if you want a realtime visual clue of what is going on.

A good rule of thumb is to try to accomplish what you want with the least amount of code. Less code = less to go wrong, and much easier to t/s or maintain.

Of course, if you are trying to obfuscate your code it brings up way more issues.
Obfuscation is a real challenge and a whole sort of mental challenge of its own.
It can be really fun to challenge other coders in obfuscation contests, but that is probably more geeky that anyone really needs to hear about :D

[MrSeanKon]

OK OK OK folks thanks, thanks again for the tips. :toast:
But I am getting busy so I will rewrite the code later. :)
Hope to be a nonFORTRAN programmer in the future! :D