Skip to content

Playable ux #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
smith11235 wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Playable ux #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b90665f
adding rvm ignroare
smith11235 May 9, 2015
ea1324a
adding my notes and index reorg
smith11235 May 9, 2015
d531f41
better frontpage
smith11235 May 9, 2015
ae1916e
reworking formss
smith11235 May 9, 2015
99f110f
min working
smith11235 May 9, 2015
ffcc630
min working
smith11235 May 9, 2015
f835731
broken but mostly cleaned up
smith11235 May 10, 2015
6842c12
saving
smith11235 May 10, 2015
ad2b7af
merged conflicts
smith11235 May 10, 2015
40a8b99
formatted
smith11235 May 10, 2015
c12f0d6
new logic merged
smith11235 May 10, 2015
f2840cd
fixed logic
smith11235 May 10, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
# Ignore bundler config.
/.bundle

.rvm
# Ignore the default SQLite database.
/db/*.sqlite3
/db/*.sqlite3-journal

# Ignore all logfiles and tempfiles.
/log/*.log
/tmp
*.sw*
/**/*.sw*
16 changes: 8 additions & 8 deletions app/controllers/application_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ class ApplicationController < ActionController::Base
# run these methods on page load
before_filter :current_user

private

# Finds the User with the ID stored in the session with the key
# :current_user_id This is a common way to handle user login in
# a Rails application; logging in sets the session value and
# logging out removes it.
helper_method :current_user

@bcwik9 bcwik9 May 10, 2015

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helper_method is deprecated. I like the previous method better for this reason

@smith11235 smith11235 May 10, 2015

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helper_method is deprecated because this should be in app/helpers/application_helper.rb.

Using a random @var in all your views and controllers is ugls.

I <3 butt stuff

@gwilbur gwilbur May 11, 2015

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean do you really need this in the view anyway? Why bother making it a helper_method?


def current_user
session[:id] ||= request.remote_ip.hash + rand(1000)
@_current_user = session[:id]
@current_user ||= unless @current_user

@bcwik9 bcwik9 May 10, 2015

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont like this rewrite. more code, removed comment, and it does the same thing

@smith11235 smith11235 May 10, 2015

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, using @_current_user all through code is bad.

Where is the @var defined? Why are you referencing it directly? A helper method is more standard which is why I rewrote all the users of @_current_user to just 'current_user'.

Then my weird format, its so less logic is executed on EVERY single call. It adds up.

@gwilbur gwilbur May 11, 2015

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part I like this refactor, but isn't the ||= and unless redundant?

@smith11235 smith11235 May 11, 2015

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not. I like htat you both call this out though. Because it is something ruby should do better.

But this is the crux. There are multiple lines of logic that could be run here.

Multiple lines of logic that I want executed only 1 time.

In the original code, why are we wasting time re-setting @_current_user = session[:id] every single time the method is called?

With my code, if @current_user is defined, it returns that with 1 if-nil check only.
Then the unless lets me have complex logic that could span multiple lines to determine what the final value will be.

Ideally this syntax would be allowed in ruby (but isnt i believe) by writing a block to be executed:

def some_var
  @some_var_i_define_once ||= {
      session[:id] ||= some-value 
  }
end

But since that formatting is invalid, you need a if/unless to open a block who's result is assigned. But, only if @some_var_i_define_once is nil.

Compare that to:

def some_var
  @some_var_i_define_once = if session[:id]
                                                      session[:id] 
                                                   else
                                                     session[:id] = some-value
                                                   end
end

This is maybe a little clearer without the weird seemingly duplicate check, but more expensive execution wise.

@gwilbur gwilbur May 11, 2015

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you do:

@current_user ||= ( session[:id] ||= request.remote_ip.hash + rand(1000) )

session[:id] ||= request.remote_ip.hash + rand(1000)
end
end

private

end
200 changes: 69 additions & 131 deletions app/controllers/games_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,74 +1,48 @@
class GamesController < ApplicationController
before_action :set_game, only: [:show, :edit, :update, :destroy]
before_action :set_game, only: [:add_player, :deal, :player_action, :show]

def add_player
unless @_current_user.nil?
set_game
state = @game.load_state
if state[:players].size < 5
unless state[:players].include?(@_current_user)
if state[:cards_in_play]
redirect_to @game, notice: 'Cant join once the game has already started'
return
end
state[:players].push @_current_user
while state[:names].include? params[:username]
# make sure username is unique by appending random numbers
params[:username] += rand(10).to_s
end
state[:names].push params[:username]
@game.save_state state
if @game.save
redirect_to @game, notice: 'Joined the game!'
else
redirect_to @game, notice: 'Failed to join the game'
end
else
redirect_to @game, notice: 'Youre already in the game'
end
else
redirect_to @game, notice: 'There are too many players in the game already'
end
if @game.state_data[:players].size > 5
redirect_to @game, notice: 'There are too many players in the game already'
elsif @game.has_player?(current_user)
redirect_to @game, notice: 'Youre already in the game'
elsif !@game.waiting_for_players?
redirect_to @game, notice: 'Cant join once the game has already started'
else
session[:player_name] = @game.add_player(current_user, params[:username])
redirect_to @game, notice: "Welcome to the game"
end
end

def deal
set_game
state = @game.load_state
if state[:players].size < 3 or state[:players].size > 5
redirect_to @game, notice: 'Must have between 3 and 5 players to start'
return
end
if state[:players].first == @_current_user
if state[:cards_in_play].nil?
@game.save_state @game.deal_cards(state)
@game.save
redirect_to @game, notice: 'Game has started!'
else
redirect_to @game, notice: 'Game has already started'
end
else
redirect_to @game, notice: 'Only the creator of the game can start it'
end
message = if @game.state_data[:players].size < 3 or @game.state_data[:players].size > 5

@gwilbur gwilbur May 11, 2015

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the format in the above method (add_player), calling redirect_to in each branch. The extra indentation here is off putting.

'Must have between 3 and 5 players to start'
elsif !@game.waiting_for_players?
'Game has already started'
elsif @game.can_deal?(current_user)
'Only the creator of the game can start it'
else
@game.deal_cards(@game.state_data)
@game.state_data[:current_status] = :in_play
@game.save!
'Game has started!'
end
redirect_to @game, notice: message
end

def player_action
set_game
state = @game.load_state

# player isnt allowed to do anything if it's not their turn
if state[:waiting_on] != @_current_user
redirect_to @game, notice: "Dude... It's not your turn"
if @game.state_data[:waiting_on] != current_user
redirect_to @game, notice: "Bro... It's not your turn"
return
end

if params[:bid]
# user is making a bid
if @game.done_bidding? state
if @game.done_bidding?
redirect_to @game, notice: 'Bidding is over BRO'
return
else
if @game.player_action(@_current_user, params[:bid])
if @game.player_action(current_user, params[:bid])
@game.save
redirect_to @game, notice: 'Placed bid, YEAAA!'
else
Expand All @@ -78,7 +52,7 @@ def player_action
else
# user is playing a card
card = Card.new(params[:suit], params[:value])
if @game.player_action(@_current_user, card)
if @game.player_action(current_user, card)
@game.save
redirect_to @game, notice: "Played a card, niceee!"
else
Expand All @@ -96,49 +70,47 @@ def index
# GET /games/1
# GET /games/1.json
def show
state = @game.load_state

@is_playing = state[:players].include?(@_current_user)
@game_started = !state[:bids].nil?
player_index = state[:players].index(@_current_user) || 0
player_index = @game.state_data[:players].index(current_user) || 0
@is_playing = @game.state_data[:player].has_key?(current_user)
@game_started = !@game.state_data[:bids].nil?

# round number
@round = state[:total_rounds] - state[:rounds_played]
@round = @game.state_data[:total_rounds] - @game.state_data[:rounds_played]

# names/scores around the board
# add in different order so the user is always on the bottom
@names = []
@round_scores = []
@game.iterate_through_list_with_start_index(player_index, state[:names]) do |name, i|
tricks_taken = (state[:tricks_taken] and state[:tricks_taken][i]) ? state[:tricks_taken][i].size : 0
bid = (state[:bids] and state[:bids][i]) ? state[:bids][i] : 'No bid yet'
@game.iterate_through_list_with_start_index(player_index, @game.state_data[:players]) do |player_id, i|
tricks_taken = (@game.state_data[:tricks_taken] and @game.state_data[:tricks_taken][i]) ? @game.state_data[:tricks_taken][i].size : 0
bid = (@game.state_data[:bids] and @game.state_data[:bids][i]) ? @game.state_data[:bids][i] : 'No bid yet'
score = 0
if state[:score] and state[:score][i]
score = state[:score][i].inject :+
if @game.state_data[:score] and @game.state_data[:score][i]
score = @game.state_data[:score][i].inject :+
end
@names.push name
@round_scores.push (name.nil?) ? nil : "Tricks taken: #{tricks_taken} | Bid: #{bid} | Score: #{score}"
@names.push @game.state_data[:names][i]
@round_scores.push (player_id.nil?) ? nil : "Tricks taken: #{tricks_taken} | Bid: #{bid} | Score: #{score}"
end

# cards that have been played
@played_cards = state[:cards_in_play]
@played_cards = @game.state_data[:cards_in_play]
if @game_started
# display cards in different order since the user is on the bottom
@played_cards = []
@game.iterate_through_list_with_start_index(player_index, state[:players]) do |player,i|
@played_cards.push state[:cards_in_play][i]
@game.iterate_through_list_with_start_index(player_index, @game.state_data[:players]) do |player,i|
@played_cards.push @game.state_data[:cards_in_play][i]
end
end

# players hand
@cards = []
if @is_playing
@cards = state[:player_hands][player_index] || @cards
if @is_playing && @game_started
@cards = @game.state_data[:player_hands][player_index] || @cards

# can't play any cards unless it's your turn
playable_cards = []
if state[:waiting_on] == @_current_user
playable_cards = @game.get_playable_cards(state[:first_suit_played], state[:player_hands][player_index])
if @game.state_data[:waiting_on] == current_user
playable_cards = @game.get_playable_cards(@game.state_data[:first_suit_played], @game.state_data[:player_hands][player_index])
end
@cards.each do |card|
if playable_cards.include? card
Expand All @@ -149,21 +121,22 @@ def show
@cards.sort! { |a,b| a.suit_order b }

# game status (ie. who we're waiting on)
if state[:waiting_on]
waiting_on_index = state[:players].index(state[:waiting_on])
@waiting_on = waiting_on_index ? state[:names][waiting_on_index] : "Table to clear"
@waiting_on = 'YOU' if @_current_user == state[:waiting_on]
@done_bidding = @game.done_bidding? state
if @game.state_data[:waiting_on]
waiting_on_index = @game.state_data[:players].index(@game.state_data[:waiting_on])
@waiting_on = @game.state_data[:names][waiting_on_index] || "Table to clear"
@waiting_on = 'YOU' if current_user == @game.state_data[:waiting_on]

@done_bidding = @game.done_bidding?
unless @done_bidding
@waiting_on += " (BIDDING)"
end
else
@waiting_on = 'Game to start'
end

# show ace of spades if game hasnt started
@trump = state[:trump_card] || Card.new('Spades', 12)
@trump = @game.state_data[:trump_card] || Card.new('Spades', 12)

# make sure show.js.erb is executed in the views folder
respond_to do |format|
format.js
Expand All @@ -176,59 +149,24 @@ def new
@game = Game.new
end

# GET /games/1/edit
def edit
end

# POST /games
# POST /games.json
def create
@game = Game.new(game_params)
@game.set_up
@game = Game.create!(game_params)
@game.state_data[:players] << current_user
@game.save!

respond_to do |format|
if @game.save
format.html { redirect_to @game, notice: 'Game was successfully created.' }
format.json { render :show, status: :created, location: @game }
else
format.html { render :new }
format.json { render json: @game.errors, status: :unprocessable_entity }
end
format.html { redirect_to @game, notice: 'Game was successfully created.' }
format.json { render :show, status: :created, location: @game }
end
end

# PATCH/PUT /games/1
# PATCH/PUT /games/1.json
def update
respond_to do |format|
if @game.update(game_params)
format.html { redirect_to @game, notice: 'Game was successfully updated.' }
format.json { render :show, status: :ok, location: @game }
else
format.html { render :edit }
format.json { render json: @game.errors, status: :unprocessable_entity }
end
end
end
private

# DELETE /games/1
# DELETE /games/1.json
def destroy
@game.destroy
respond_to do |format|
format.html { redirect_to games_url, notice: 'Game was successfully destroyed.' }
format.json { head :no_content }
end
def set_game
@game = Game.find(params[:id])
end

private
# Use callbacks to share common setup or constraints between actions.
def set_game
@game = Game.find(params[:id])
end

# Never trust parameters from the scary internet, only allow the white list through.
def game_params
params.require(:game).permit(:name, :state)
end
def game_params
params.require(:game).permit(:name, :state)
end
end
Loading