[DO NOT MERGE] Final project code review

.gitignore

@@ -226,4 +226,5 @@ build/
 dist/
 node_modules/
 aws-exports.js
-awsconfiguration.json
+awsconfiguration.json
+.idea/caches/

README.md

@@ -3,7 +3,7 @@
 ## APK
 To download and play Zombie Tag click the zombie below  
 
-[![Build Status](https://github.com/JavaAwesome/JavaTag/blob/Dev/app/src/main/res/mipmap-hdpi/ic_launcher_round.png)](https://github.com/JavaAwesome/JavaTag/blob/Dev/zombietag.apk)
+[![Download App](https://github.com/JavaAwesome/JavaTag/blob/Dev/app/src/main/res/mipmap-hdpi/ic_launcher_round.png)](https://github.com/JavaAwesome/JavaTag/blob/Dev/zombietag.apk)
 
 ## Team Members
 * Ahren Swett
@@ -23,6 +23,8 @@ To make an android app where players can see one another on a map and tag one an
 
 ## Instructions to run the app on your phone
 
+<!-- Actual instructions go here, please! -->
+
 ### Contribute
 No contribution guidelines at this point.
 

app/src/main/java/com/javaawesome/tag/MainActivity.java

@@ -53,7 +53,8 @@
 import static android.Manifest.permission.ACCESS_FINE_LOCATION;
 
 public class MainActivity extends AppCompatActivity implements SessionAdapter.OnSessionInteractionListener {
-    protected static String photoBucketPath = "https://javatag091c7e33ab0441e4bdf34cbdf68d2bd1-local.s3-us-west-2.amazonaws.com/";
+    // final is a good idea for a variable like this that you don't ever want to accidentally change
+    protected static final String photoBucketPath = "https://javatag091c7e33ab0441e4bdf34cbdf68d2bd1-local.s3-us-west-2.amazonaws.com/";
     private final String TAG = "javatag";
     RecyclerView recyclerNearbySessions;
     SessionAdapter sessionAdapter;
@@ -107,7 +108,7 @@ public void onError(Exception e) {
         sessions = new LinkedList<>();
 
         // initialize recycler view to display nearby game sessions
-        // TODO: have recycler view filter sessions by distance to user
+        // I think this was completed with the call to filter--so this is an out of date comment.
         recyclerNearbySessions = findViewById(R.id.recycler_nearby_sessions);
         recyclerNearbySessions.setLayoutManager(new LinearLayoutManager(this));
         this.sessionAdapter = new SessionAdapter(this.sessions, this);
@@ -119,18 +120,20 @@ protected void onResume() {
         super.onResume();
         Log.i(TAG, "onresume called");
         if (checkGpsStatus()) {
-//            getCurrentUserLocation();
             checkIfPlayerAlreadyExistInDatabase();
         } else {
             buildAlertMessageNoGps();
         }
     }
 
     // Create new game session and go to map page
-    public void goToMap(View view) {
+    // Called when the new session button is tapped, so it should probably have a clearer name!
+    public void createAndGoToNewSession(View view) {
         // TODO: check if player already exist in the database
+        // didn't we check if the player existed in onResume? Why do we need to call it again now?
+        // Or is this just a leftover todo?
         EditText sessionName = findViewById(R.id.editText_session_name);
-        Log.i(TAG, "goToMap: "+sessionName.getText());
+        Log.i(TAG, "createAndGoToNewSession: "+sessionName.getText());
         if(sessionName.getText().length()>0) {
             CreateSessionInput input = CreateSessionInput.builder()
                     .title(sessionName.getText().toString())
@@ -143,10 +146,7 @@ public void goToMap(View view) {
                 @Override
                 public void onResponse(@Nonnull Response<CreateSessionMutation.Data> response) {
                     sessionId = response.data().createSession().id();
-                    Intent goToMapIntent = new Intent(MainActivity.this, MapsActivity.class);
-                    goToMapIntent.putExtra("sessionId", sessionId);
-                    goToMapIntent.putExtra("userID", playerId);
-                    MainActivity.this.startActivity(goToMapIntent);
+                    joinExistingGameSession(sessionId);
                 }
 
                 @Override
@@ -164,14 +164,7 @@ public void goToUserPage(View view){
         Intent goToUserPage = new Intent(this, UserProfile.class);
         this.startActivity(goToUserPage.putExtra("playerId",playerId));
     }
-
-    //////// TEST BUTTON /////
-    public void onTestyClick(View view) {
-        startActivity(new Intent(MainActivity.this, NotificationActivity.class));
-    }
-
-
-
+    // dead code 💀
 
     // Direct users to sign in page
     private void signInUser() {
@@ -198,42 +191,15 @@ public void signoutCurrentUser(View view) {
     }
 
     // onclick method for button to join existing game sessions
+    // If you make this a bit more generic, it'll work for new sessions and existing ones!
     @Override
-    public void joinExistingGameSession(ListSessionsQuery.Item session) {
+    public void joinExistingGameSession(String sessionId) {
         Intent goToMapIntent = new Intent(this, MapsActivity.class);
-        goToMapIntent.putExtra("sessionId", session.id());
+        goToMapIntent.putExtra("sessionId", sessionId);
         goToMapIntent.putExtra("userID", playerId);
         this.startActivity(goToMapIntent);
     }
 
-//    @Override
-//    public void addPlayerToChosenGame(final ListSessionsQuery.Item session) {
-////        Query
-//        CreatePlayerInput playerInput = CreatePlayerInput.builder()
-//                .playerSessionId(session.id())
-//                .isIt(false)
-//                .lat(currentUserLocation.latitude)
-//                .lon(currentUserLocation.longitude)
-//                .username(AWSMobileClient.getInstance().getUsername())
-//                .build();
-//        CreatePlayerMutation createPlayerMutation = CreatePlayerMutation.builder().input(playerInput).build();
-//        awsAppSyncClient.mutate(createPlayerMutation).enqueue((new GraphQLCall.Callback<CreatePlayerMutation.Data>() {
-//            @Override
-//            public void onResponse(@Nonnull Response<CreatePlayerMutation.Data> response) {
-//                String userID = response.data().createPlayer().id();
-//                Log.i(TAG, "player mutation happened! ... inside of a session mutation");
-//                Intent goToMapIntent = new Intent(MainActivity.this, MapsActivity.class);
-//                goToMapIntent.putExtra("sessionId", session.id());
-//                goToMapIntent.putExtra("userID", userID);
-//                Log.i("veach", session.id() + "\n" +userID);
-//            }
-//            @Override
-//            public void onFailure(@Nonnull ApolloException e) {
-//                Log.i(TAG, "mutation of player failed, boohoo!");
-//            }
-//        }));
-//    }
-
     // get all sessions
     private void queryAllSessions() {
         Log.i(TAG, "query all sessions");

app/src/main/java/com/javaawesome/tag/MapsActivity.java

@@ -68,9 +68,7 @@ public class MapsActivity extends FragmentActivity implements OnMapReadyCallback
     GetSessionQuery.GetSession currentSession;
 
     LatLng startingPoint;
-    final static long REFRESHRATE = 3*1000;
-    final static int SUBJECT = 0;
-    Handler locationHandler;
+    // Lots of leftover, unused variables around in this code; I wish they'd been removed.
     LocationCallback mLocationCallback;
     private FusedLocationProviderClient mFusedLocationClient;
     final private int tagDistance = 50;
@@ -81,6 +79,7 @@ public class MapsActivity extends FragmentActivity implements OnMapReadyCallback
     BitmapDescriptor playerpin;
     List<Player> players;
     private final String TAG = "javatag";
+    // The inconsistency of playerID vs sessionId (in capitalization) is annoying.
     String playerID;
     Player player;
     String sessionId;
@@ -113,15 +112,17 @@ protected void onCreate(Bundle savedInstanceState) {
         Log.i(TAG, "Session ID for map is: " + sessionId + "the player Id is " + playerID);
 
         associateUserWithSession();
-
-        // Pull user ID from MainActivity
-        // If player comes from the recyclerView it will come through as null so we will create a new player
-        // Else the player created the game and we will query the player object
-//        if (playerID == null) {
-//            createPlayer();
-//        } else {
-//            queryForPlayerObject();
-//        }
+        // so what this actually starts is:
+        // associateUserWithSession
+        // makes request to dynamo
+        // then calls queryForSelectedSession
+        // which simultaneously calls initializeMarkers and queryForPlayerObject
+        // queryForPlayerObject gets the current player's data, and sets it to be part of the players list
+        // initializeMarkers does the same exact work, and the current player should already be in that players list!
+        // this is why I was getting "two" players in my single player game!
+        // I think this is rendering the current player twice!!!
+        // But regardless, this is QUITE complicated and all kicked off by calling associateUserWithSession here; that feels
+        // like a lot of work for a method with such a short name! "renderGame" might be more accurate than "associateUserWithSession"!
 
         //Stuff doesn't start running until the map is ready in onMapReady(Map stuff)
         mLocationCallback = new LocationCallback() {
@@ -133,7 +134,9 @@ public void onLocationResult(LocationResult locationResult) {
                     return;
                 }
 
-                updateMarkerAndCircleForAllPlayers(players);
+                // I think it's odd that we call this here, instead of when updates come in from
+                // our subscription to Dynamo.
+                updateMarkerAndCircleForAllPlayers();
                 sendUserLocationQuery(locationResult);
 
             }
@@ -388,7 +391,8 @@ private LocationRequest getLocationRequest() {
     }
 
     // Creates markers and circles for each player in the list for that session
-    private void initializeMarkersAndCirclesForPlayers(List<Player> players) {
+    // Since this only ever operates on the instance variable, it doesn't need a param.
+    private void initializeMarkersAndCirclesForPlayers() {
         Log.i(TAG, "made it to initialized markers");
         for(Player player: players) {
             Marker marker = mMap.addMarker(new MarkerOptions()
@@ -424,8 +428,8 @@ private void initializeMarkersAndCirclesForPlayers(List<Player> players) {
                 .strokeWidth(5));
     }
 
-
-    private void updateMarkerAndCircleForAllPlayers(List<Player> players) {
+    // Since this only ever operates on the instance variable, it doesn't need a parameter.
+    private void updateMarkerAndCircleForAllPlayers() {
         Log.i(TAG, "updating markers");
         Log.i(TAG, "How many players? " + players.size());
 
@@ -451,15 +455,9 @@ private void updateMarkerAndCircleForAllPlayers(List<Player> players) {
                 Log.i(TAG, "In the updateMarkerAndCircleForAllPlayers");
                 playersJustGotTagged.add(player);
 
-//                player.getMarker().setIcon(zombiepin);
-//                player.getCircle().setStrokeColor(itColor);
-
-//                mMap.addCircle(player.getCircle());
+                // This thing with a bunch of commented out lines with the occasional close curly
+                // brace in the middle is quite annoying! I'd prefer to just remove these lines.
             }
-//            else {
-//                player.getMarker().setIcon(playerpin);
-//                player.getCircle().setStrokeColor(notItColor);
-//            }
         }
         //TODO add the player instance is it update, only updates the player list so far
         for(Player player : players){
@@ -545,39 +543,6 @@ private void queryForSelectedSession(String sessionId) {
                 .enqueue(getSessionCallBack);
     }
 
-    // Make a Player
-    private void createPlayer() {
-        Log.i(TAG, "Making a player with " + sessionId + " " + startingPoint.toString());
-        CreatePlayerInput input = CreatePlayerInput.builder()
-                .playerSessionId(sessionId)
-                .lat(startingPoint.latitude)
-                .lon(startingPoint.longitude)
-                .username(AWSMobileClient.getInstance().getUsername())
-                .isIt(false)
-                .build();
-        CreatePlayerMutation createPlayerMutation = CreatePlayerMutation.builder().input(input).build();
-
-        awsAppSyncClient.mutate(createPlayerMutation).enqueue(new GraphQLCall.Callback<CreatePlayerMutation.Data>() {
-            @Override
-            public void onResponse(@Nonnull Response<CreatePlayerMutation.Data> response) {
-                Log.i(TAG, "made it to creating a new player");
-                playerID = response.data().createPlayer().id();
-                player = new Player();
-                player.setId(playerID);
-                player.setIt(false);
-                player.setUsername(AWSMobileClient.getInstance().getUsername());
-                List<LatLng> bananas = new LinkedList<>();
-                bananas.add(startingPoint);
-                player.setLocations(bananas);
-            }
-
-            @Override
-            public void onFailure(@Nonnull ApolloException e) {
-                Log.e(TAG, "couldn't make a new player");
-            }
-        });
-    }
-
     // Query for Player
     private void queryForPlayerObject() {
         GetPlayerQuery query = GetPlayerQuery.builder().id(playerID).build();
@@ -642,13 +607,8 @@ public void onResponse(@Nonnull final Response<GetSessionQuery.Data> response) {
 
             //once the session ID and starting loc are in place, then make the first player.
             queryForPlayerObject();
-//            if (playerID == null) {
-//                createPlayer();
-//            } else {
-//                queryForPlayerObject();
-//            }
 
-            Log.i(TAG, "Made it to the after the if/else within getSessionCallBack");
+            // there is no if/else here... this is defintitely an out of date comment
             //converting from GetSessionItems to players
             players = playerConverter(currentSession.players().items());
 
@@ -663,7 +623,7 @@ public void handleMessage(Message inputMessage){
                     } catch (InterruptedException e) {
                         e.printStackTrace();
                     }
-                    initializeMarkersAndCirclesForPlayers(players);
+                    initializeMarkersAndCirclesForPlayers();
 
                 }
             };
@@ -685,10 +645,4 @@ private List<Player> playerConverter(List<GetSessionQuery.Item> incomingList){
         return outGoingList;
     };
 
-    // TODO: Build onDestroy that deletes user data from DB
-//    @Override
-//    protected void onDestroy() {
-//        super.onDestroy();
-//    }
-
 }

app/src/main/java/com/javaawesome/tag/SessionAdapter.java

@@ -1,8 +1,6 @@
 package com.javaawesome.tag;
 
 import android.content.DialogInterface;
-import android.content.Intent;
-import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -40,8 +38,7 @@ public void onClick(View view) {
                     .setPositiveButton("Yes", new DialogInterface.OnClickListener() {
                         @Override
                         public void onClick(DialogInterface dialog, int which) {
-//                            listener.addPlayerToChosenGame(holder.session);
-                            listener.joinExistingGameSession(holder.session);
+                            listener.joinExistingGameSession(holder.session.id());
                             //TODO:  Save the users ID to the database based on the session that they clicked
                         }
                     })
@@ -55,12 +52,10 @@ public static class SessionViewHolder extends RecyclerView.ViewHolder {
 
         ListSessionsQuery.Item session;
         TextView sessionTitle;
-//        TextView numberOfPlayers;
 
         public SessionViewHolder(@NonNull View itemView) {
             super(itemView);
             this.sessionTitle = itemView.findViewById(R.id.session_title);
-//            this.numberOfPlayers = itemView.findViewById(R.id.session_total_players);
         }
     }
 
@@ -69,7 +64,6 @@ public void onBindViewHolder(@NonNull SessionAdapter.SessionViewHolder holder, i
         ListSessionsQuery.Item sessionAtPosition = this.sessions.get(position);
         holder.session = sessionAtPosition;
         holder.sessionTitle.setText(sessionAtPosition.title());
-//        holder.numberOfPlayers.setText("Population: " + sessionAtPosition.players());
     }
 
     @Override
@@ -78,7 +72,6 @@ public int getItemCount() {
     }
 
     public static interface OnSessionInteractionListener {
-        public void joinExistingGameSession(ListSessionsQuery.Item session);
-//        public void addPlayerToChosenGame(ListSessionsQuery.Item session);
+        public void joinExistingGameSession(String sessionId);
     }
 }