delint_d2networking (#599)

* delint_d2networking

not sure what to do about lint error G110 on calls to `io.Copy`, warns
about gzip compression bomb possibility, leaving those.

all todo's have been left.

* removed duplicate const

d2core/d2map/d2mapentity/player.go

@@ -15,8 +15,8 @@ import (
 type Player struct {
 	mapEntity
 	composite *d2asset.Composite
-	Equipment d2inventory.CharacterEquipment
+	Equipment *d2inventory.CharacterEquipment
-	Stats     d2hero.HeroStatsState
+	Stats     *d2hero.HeroStatsState
 	Class     d2enum.Hero
 	Id        string
 	name      string
@@ -34,7 +34,8 @@ var baseWalkSpeed = 6.0
 var baseRunSpeed = 9.0
 
 // CreatePlayer creates a new player entity and returns a pointer to it.
-func CreatePlayer(id, name string, x, y int, direction int, heroType d2enum.Hero, stats d2hero.HeroStatsState, equipment d2inventory.CharacterEquipment) *Player {
+func CreatePlayer(id, name string, x, y int, direction int, heroType d2enum.Hero,
+	stats *d2hero.HeroStatsState, equipment *d2inventory.CharacterEquipment) *Player {
 	layerEquipment := &[d2enum.CompositeTypeMax]string{
 		d2enum.CompositeTypeHead:      equipment.Head.GetArmorClass(),
 		d2enum.CompositeTypeTorso:     equipment.Torso.GetArmorClass(),

d2game/d2gamescreen/character_select.go

@@ -203,11 +203,15 @@ func (v *CharacterSelect) updateCharacterBoxes() {
 		v.characterNameLabel[i].SetText(v.gameStates[idx].HeroName)
 		v.characterStatsLabel[i].SetText("Level 1 " + v.gameStates[idx].HeroType.String())
 		v.characterExpLabel[i].SetText(expText)
+
+		heroType := v.gameStates[idx].HeroType
+		equipment := d2inventory.HeroObjects[heroType]
+
 		// TODO: Generate or load the object from the actual player data...
 		v.characterImage[i] = d2mapentity.CreatePlayer("", "", 0, 0, 0,
 			v.gameStates[idx].HeroType,
-			*v.gameStates[idx].Stats,
+			v.gameStates[idx].Stats,
-			d2inventory.HeroObjects[v.gameStates[idx].HeroType],
+			&equipment,
 		)
 	}
 }

d2game/d2gamescreen/game.go

@@ -153,7 +153,7 @@ func (v *Game) Advance(tickTime float64) error {
 
 func (v *Game) bindGameControls() {
 	for _, player := range v.gameClient.Players {
-		if player.Id != v.gameClient.PlayerId {
+		if player.Id != v.gameClient.PlayerID {
 			continue
 		}
 
@@ -173,19 +173,19 @@ func (v *Game) bindGameControls() {
 func (v *Game) OnPlayerMove(x, y float64) {
 	worldPosition := v.localPlayer.Position.World()
 
-	err := v.gameClient.SendPacketToServer(d2netpacket.CreateMovePlayerPacket(v.gameClient.PlayerId, worldPosition.X(), worldPosition.Y(), x, y))
+	err := v.gameClient.SendPacketToServer(d2netpacket.CreateMovePlayerPacket(v.gameClient.PlayerID, worldPosition.X(), worldPosition.Y(), x, y))
 	if err != nil {
-		fmt.Printf("failed to send MovePlayer packet to the server, playerId: %s, x: %g, x: %g\n", v.gameClient.PlayerId, x, y)
+		fmt.Printf("failed to send MovePlayer packet to the server, playerId: %s, x: %g, x: %g\n", v.gameClient.PlayerID, x, y)
 	}
 }
 
 // OnPlayerCast sends the casting skill action to the server
 func (v *Game) OnPlayerCast(missileID int, targetX, targetY float64) {
-	err := v.gameClient.SendPacketToServer(d2netpacket.CreateCastPacket(v.gameClient.PlayerId, missileID, targetX, targetY))
+	err := v.gameClient.SendPacketToServer(d2netpacket.CreateCastPacket(v.gameClient.PlayerID, missileID, targetX, targetY))
 	if err != nil {
 		fmt.Printf(
 			"failed to send CastSkill packet to the server, playerId: %s, missileId: %d, x: %g, x: %g\n",
-			v.gameClient.PlayerId, missileID, targetX, targetY,
+			v.gameClient.PlayerID, missileID, targetX, targetY,
 		)
 	}
 }

d2game/d2player/hero_stats_panel.go

@@ -84,7 +84,7 @@ type HeroStatsPanel struct {
 }
 
 func NewHeroStatsPanel(renderer d2interface.Renderer, heroName string, heroClass d2enum.Hero,
-	heroState d2hero.HeroStatsState) *HeroStatsPanel {
+	heroState *d2hero.HeroStatsState) *HeroStatsPanel {
 	originX := 0
 	originY := 0
 
@@ -92,7 +92,7 @@ func NewHeroStatsPanel(renderer d2interface.Renderer, heroName string, heroClass
 		renderer:  renderer,
 		originX:   originX,
 		originY:   originY,
-		heroState: &heroState,
+		heroState: heroState,
 		heroName:  heroName,
 		heroClass: heroClass,
 		labels:    &StatsPanelLabels{},

d2networking/d2client/d2clientconnectiontype/doc.go

@@ -1,3 +1,2 @@
-// Package d2clientconnectiontype provides types
+// Package d2clientconnectiontype provides types for client connections
-// for client connections
 package d2clientconnectiontype

d2networking/d2client/d2localclient/local_client_connection.go

@@ -1,26 +1,27 @@
 package d2localclient
 
 import (
+	uuid "github.com/satori/go.uuid"
+
 	"github.com/OpenDiablo2/OpenDiablo2/d2game/d2player"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2client/d2clientconnectiontype"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2server"
-	uuid "github.com/satori/go.uuid"
 )
 
 // LocalClientConnection is the implementation of ClientConnection
 // for a local client.
 type LocalClientConnection struct {
 	clientListener    d2networking.ClientListener // The game client
-	uniqueId          string                      // Unique ID generated on construction
+	uniqueID          string                      // Unique ID generated on construction
 	openNetworkServer bool                        // True if this is a server
 	playerState       *d2player.PlayerState       // Local player state
 }
 
-// GetUniqueId returns LocalClientConnection.uniqueId.
+// GetUniqueID returns LocalClientConnection.uniqueID.
-func (l LocalClientConnection) GetUniqueId() string {
+func (l LocalClientConnection) GetUniqueID() string {
-	return l.uniqueId
+	return l.uniqueID
 }
 
 // GetConnectionType returns an enum representing the connection type.
@@ -38,7 +39,7 @@ func (l *LocalClientConnection) SendPacketToClient(packet d2netpacket.NetPacket)
 // a pointer to it.
 func Create(openNetworkServer bool) *LocalClientConnection {
 	result := &LocalClientConnection{
-		uniqueId:          uuid.NewV4().String(),
+		uniqueID:          uuid.NewV4().String(),
 		openNetworkServer: openNetworkServer,
 	}
 
@@ -46,7 +47,7 @@ func Create(openNetworkServer bool) *LocalClientConnection {
 }
 
 // Open creates a new GameServer, runs the server and connects this client to it.
-func (l *LocalClientConnection) Open(_ string, saveFilePath string) error {
+func (l *LocalClientConnection) Open(_, saveFilePath string) error {
 	l.SetPlayerState(d2player.LoadPlayerState(saveFilePath))
 	d2server.Create(l.openNetworkServer)
 

d2networking/d2client/d2remoteclient/remote_client_connection.go

@@ -10,14 +10,13 @@ import (
 	"net"
 	"strings"
 
+	uuid "github.com/satori/go.uuid"
+
 	"github.com/OpenDiablo2/OpenDiablo2/d2game/d2player"
-
-	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2client/d2clientconnectiontype"
-
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking"
+	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2client/d2clientconnectiontype"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket/d2netpackettype"
-	uuid "github.com/satori/go.uuid"
 )
 
 // RemoteClientConnection is the implementation of ClientConnection
@@ -126,8 +125,8 @@ func (r *RemoteClientConnection) SendPacketToServer(packet d2netpacket.NetPacket
 		return fmt.Errorf("remoteClientConnection: attempted to send empty %v packet body", packet.PacketType)
 	}
 
-	if err := writer.Close(); err != nil {
+	if writeErr := writer.Close(); writeErr != nil {
-		return err
+		return writeErr
 	}
 
 	if _, err = r.udpConnection.Write(buff.Bytes()); err != nil {

d2networking/d2client/game_client.go

@@ -20,15 +20,19 @@ import (
 	"github.com/OpenDiablo2/OpenDiablo2/d2script"
 )
 
+const (
+	numSubtilesPerTile = 5
+)
+
 // GameClient manages a connection to d2server.GameServer
-// and keeps a synchronised copy of the map and entities.
+// and keeps a synchronized copy of the map and entities.
 type GameClient struct {
 	clientConnection ServerConnection                            // Abstract local/remote connection
 	connectionType   d2clientconnectiontype.ClientConnectionType // Type of connection (local or remote)
 	scriptEngine     *d2script.ScriptEngine
 	GameState        *d2player.PlayerState          // local player state
 	MapEngine        *d2mapengine.MapEngine         // Map and entities
-	PlayerId         string                         // ID of the local player
+	PlayerID         string                         // ID of the local player
 	Players          map[string]*d2mapentity.Player // IDs of the other players
 	Seed             int64                          // Map seed
 	RegenMap         bool                           // Regenerate tile cache on render (map has changed)
@@ -53,18 +57,21 @@ func Create(connectionType d2clientconnectiontype.ClientConnectionType, scriptEn
 	default:
 		return nil, fmt.Errorf("unknown client connection type specified: %d", connectionType)
 	}
+
 	result.clientConnection.SetClientListener(result)
+
 	return result, nil
 }
 
 // Open creates the server and connects to it if the client is local.
 // If the client is remote it sends a PlayerConnectionRequestPacket to the
 // server (see d2netpacket).
-func (g *GameClient) Open(connectionString string, saveFilePath string) error {
+func (g *GameClient) Open(connectionString, saveFilePath string) error {
 	switch g.connectionType {
 	case d2clientconnectiontype.LANServer, d2clientconnectiontype.Local:
 		g.scriptEngine.AllowEval()
 	}
+
 	return g.clientConnection.Open(connectionString, saveFilePath)
 }
 
@@ -75,6 +82,7 @@ func (g *GameClient) Close() error {
 	case d2clientconnectiontype.LANServer, d2clientconnectiontype.Local:
 		g.scriptEngine.DisallowEval()
 	}
+
 	return g.clientConnection.Close()
 }
 
@@ -88,77 +96,27 @@ func (g *GameClient) Destroy() error {
 func (g *GameClient) OnPacketReceived(packet d2netpacket.NetPacket) error {
 	switch packet.PacketType {
 	case d2netpackettype.GenerateMap:
-		mapData := packet.PacketData.(d2netpacket.GenerateMapPacket)
+		if err := g.handleGenerateMapPacket(packet); err != nil {
-		switch mapData.RegionType {
+			return err
-		case d2enum.RegionAct1Town:
+		}
-			d2mapgen.GenerateAct1Overworld(g.MapEngine)
+	case d2netpackettype.UpdateServerInfo:
-		}
+		if err := g.handleUpdateServerInfoPacket(packet); err != nil {
-		g.RegenMap = true
+			return err
-	case d2netpackettype.UpdateServerInfo:
+		}
-		serverInfo := packet.PacketData.(d2netpacket.UpdateServerInfoPacket)
+	case d2netpackettype.AddPlayer:
-		g.MapEngine.SetSeed(serverInfo.Seed)
+		if err := g.handleAddPlayerPacket(packet); err != nil {
-		g.PlayerId = serverInfo.PlayerId
+			return err
-		g.Seed = serverInfo.Seed
+		}
-		log.Printf("Player id set to %s", serverInfo.PlayerId)
+	case d2netpackettype.MovePlayer:
-	case d2netpackettype.AddPlayer:
+		if err := g.handleMovePlayerPacket(packet); err != nil {
-		player := packet.PacketData.(d2netpacket.AddPlayerPacket)
+			return err
-		newPlayer := d2mapentity.CreatePlayer(player.Id, player.Name, player.X, player.Y, 0, player.HeroType, player.Stats, player.Equipment)
+		}
-		g.Players[newPlayer.Id] = newPlayer
+	case d2netpackettype.CastSkill:
-		g.MapEngine.AddEntity(newPlayer)
+		if err := g.handleCastSkillPacket(packet); err != nil {
-	case d2netpackettype.MovePlayer:
-		movePlayer := packet.PacketData.(d2netpacket.MovePlayerPacket)
-		player := g.Players[movePlayer.PlayerId]
-		path, _, _ := g.MapEngine.PathFind(movePlayer.StartX, movePlayer.StartY, movePlayer.DestX, movePlayer.DestY)
-		if len(path) > 0 {
-			player.SetPath(path, func() {
-				tilePosition := player.Position.Tile()
-				tile := g.MapEngine.TileAt(int(tilePosition.X()), int(tilePosition.Y()))
-				if tile == nil {
-					return
-				}
-
-				regionType := tile.RegionType
-				if regionType == d2enum.RegionAct1Town {
-					player.SetIsInTown(true)
-				} else {
-					player.SetIsInTown(false)
-				}
-				err := player.SetAnimationMode(player.GetAnimationMode())
-				if err != nil {
-					log.Printf("GameClient: error setting animation mode for player %s: %s", player.Id, err)
-				}
-			})
-		}
-	case d2netpackettype.CastSkill:
-		playerCast := packet.PacketData.(d2netpacket.CastPacket)
-		player := g.Players[playerCast.SourceEntityID]
-		player.SetCasting()
-		player.ClearPath()
-		// currently hardcoded to missile skill
-		missile, err := d2mapentity.CreateMissile(
-			int(player.Position.X()),
-			int(player.Position.Y()),
-			d2datadict.Missiles[playerCast.SkillID],
-		)
-		if err != nil {
 			return err
 		}
-
-		rads := d2common.GetRadiansBetween(
-			player.Position.X(),
-			player.Position.Y(),
-			playerCast.TargetX*5,
-			playerCast.TargetY*5,
-		)
-
-		missile.SetRadians(rads, func() {
-			g.MapEngine.RemoveEntity(missile)
-		})
-
-		g.MapEngine.AddEntity(missile)
 	case d2netpackettype.Ping:
-		err := g.clientConnection.SendPacketToServer(d2netpacket.CreatePongPacket(g.PlayerId))
+		if err := g.handlePingPacket(); err != nil {
-		if err != nil {
 			log.Printf("GameClient: error responding to server ping: %s", err)
 		}
 	case d2netpackettype.PlayerDisconnectionNotification:
@@ -171,6 +129,7 @@ func (g *GameClient) OnPacketReceived(packet d2netpacket.NetPacket) error {
 	default:
 		log.Fatalf("Invalid packet type: %d", packet.PacketType)
 	}
+
 	return nil
 }
 
@@ -179,3 +138,113 @@ func (g *GameClient) OnPacketReceived(packet d2netpacket.NetPacket) error {
 func (g *GameClient) SendPacketToServer(packet d2netpacket.NetPacket) error {
 	return g.clientConnection.SendPacketToServer(packet)
 }
+
+func (g *GameClient) handleGenerateMapPacket(packet d2netpacket.NetPacket) error {
+	mapData := packet.PacketData.(d2netpacket.GenerateMapPacket)
+
+	if mapData.RegionType == d2enum.RegionAct1Town {
+		d2mapgen.GenerateAct1Overworld(g.MapEngine)
+	}
+
+	g.RegenMap = true
+
+	return nil
+}
+
+func (g *GameClient) handleUpdateServerInfoPacket(packet d2netpacket.NetPacket) error {
+	serverInfo := packet.PacketData.(d2netpacket.UpdateServerInfoPacket)
+	g.MapEngine.SetSeed(serverInfo.Seed)
+	g.PlayerID = serverInfo.PlayerID
+	g.Seed = serverInfo.Seed
+	log.Printf("Player id set to %s", serverInfo.PlayerID)
+
+	return nil
+}
+
+func (g *GameClient) handleAddPlayerPacket(packet d2netpacket.NetPacket) error {
+	player := packet.PacketData.(d2netpacket.AddPlayerPacket)
+	newPlayer := d2mapentity.CreatePlayer(player.ID, player.Name, player.X, player.Y, 0,
+		player.HeroType, player.Stats, &player.Equipment)
+
+	g.Players[newPlayer.Id] = newPlayer
+	g.MapEngine.AddEntity(newPlayer)
+
+	return nil
+}
+
+func (g *GameClient) handleMovePlayerPacket(packet d2netpacket.NetPacket) error {
+	movePlayer := packet.PacketData.(d2netpacket.MovePlayerPacket)
+	player := g.Players[movePlayer.PlayerID]
+	path, _, _ := g.MapEngine.PathFind(movePlayer.StartX, movePlayer.StartY, movePlayer.DestX, movePlayer.DestY)
+
+	if len(path) > 0 {
+		player.SetPath(path, func() {
+			tilePosition := player.Position.Tile()
+			tile := g.MapEngine.TileAt(int(tilePosition.X()), int(tilePosition.Y()))
+
+			if tile == nil {
+				return
+			}
+
+			regionType := tile.RegionType
+			if regionType == d2enum.RegionAct1Town {
+				player.SetIsInTown(true)
+			} else {
+				player.SetIsInTown(false)
+			}
+
+			err := player.SetAnimationMode(player.GetAnimationMode())
+
+			if err != nil {
+				log.Printf("GameClient: error setting animation mode for player %s: %s", player.Id, err)
+			}
+		})
+	}
+
+	return nil
+}
+
+func (g *GameClient) handleCastSkillPacket(packet d2netpacket.NetPacket) error {
+	playerCast := packet.PacketData.(d2netpacket.CastPacket)
+	player := g.Players[playerCast.SourceEntityID]
+
+	player.SetCasting()
+	player.ClearPath()
+
+	// currently hardcoded to missile skill
+	missile, err := d2mapentity.CreateMissile(
+		int(player.Position.X()),
+		int(player.Position.Y()),
+		d2datadict.Missiles[playerCast.SkillID],
+	)
+
+	if err != nil {
+		return err
+	}
+
+	rads := d2common.GetRadiansBetween(
+		player.Position.X(),
+		player.Position.Y(),
+		playerCast.TargetX*numSubtilesPerTile,
+		playerCast.TargetY*numSubtilesPerTile,
+	)
+
+	missile.SetRadians(rads, func() {
+		g.MapEngine.RemoveEntity(missile)
+	})
+
+	g.MapEngine.AddEntity(missile)
+
+	return nil
+}
+
+func (g *GameClient) handlePingPacket() error {
+	pongPacket := d2netpacket.CreatePongPacket(g.PlayerID)
+	err := g.clientConnection.SendPacketToServer(pongPacket)
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

d2networking/d2netpacket/d2netpackettype/message_type.go

@@ -1,4 +1,3 @@
-// Package d2netpackettype defines the enumerable NetPacketType.
 package d2netpackettype
 
 // NetPacketType is an enum referring to all packet types in package

d2networking/d2netpacket/doc.go

@@ -0,0 +1,2 @@
+// Package d2netpacket provides all of the different types of packets
+package d2netpacket

d2networking/d2netpacket/packet_add_player.go

@@ -11,22 +11,23 @@ import (
 // It is sent by the server to create the entity for a newly connected
 // player on a client.
 type AddPlayerPacket struct {
-	Id        string                         `json:"id"`
+	ID        string                         `json:"id"`
 	Name      string                         `json:"name"`
 	X         int                            `json:"x"`
 	Y         int                            `json:"y"`
 	HeroType  d2enum.Hero                    `json:"hero"`
 	Equipment d2inventory.CharacterEquipment `json:"equipment"`
-	Stats     d2hero.HeroStatsState          `json:"heroStats"`
+	Stats     *d2hero.HeroStatsState         `json:"heroStats"`
 }
 
 // CreateAddPlayerPacket returns a NetPacket which declares an
 // AddPlayerPacket with the data in given parameters.
-func CreateAddPlayerPacket(id, name string, x, y int, heroType d2enum.Hero, stats d2hero.HeroStatsState, equipment d2inventory.CharacterEquipment) NetPacket {
+func CreateAddPlayerPacket(id, name string, x, y int, heroType d2enum.Hero,
+	stats *d2hero.HeroStatsState, equipment d2inventory.CharacterEquipment) NetPacket {
 	return NetPacket{
 		PacketType: d2netpackettype.AddPlayer,
 		PacketData: AddPlayerPacket{
-			Id:        id,
+			ID:        id,
 			Name:      name,
 			X:         x,
 			Y:         y,

d2networking/d2netpacket/packet_generate_map.go

@@ -21,5 +21,4 @@ func CreateGenerateMapPacket(regionType d2enum.RegionIdType) NetPacket {
 			RegionType: regionType,
 		},
 	}
-
 }

d2networking/d2netpacket/packet_move_player.go

@@ -6,7 +6,7 @@ import "github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket/d2netpackett
 // It is sent by the server to move a player entity on a client.
 // TODO: Need to handle being on different maps
 type MovePlayerPacket struct {
-	PlayerId string  `json:"playerId"`
+	PlayerID string  `json:"playerId"`
 	StartX   float64 `json:"startX"`
 	StartY   float64 `json:"startY"`
 	DestX    float64 `json:"destX"`
@@ -15,11 +15,11 @@ type MovePlayerPacket struct {
 
 // CreateMovePlayerPacket returns a NetPacket which declares a MovePlayerPacket
 // with the given ID and movement command.
-func CreateMovePlayerPacket(playerId string, startX, startY, destX, destY float64) NetPacket {
+func CreateMovePlayerPacket(playerID string, startX, startY, destX, destY float64) NetPacket {
 	return NetPacket{
 		PacketType: d2netpackettype.MovePlayer,
 		PacketData: MovePlayerPacket{
-			PlayerId: playerId,
+			PlayerID: playerID,
 			StartX:   startX,
 			StartY:   startY,
 			DestX:    destX,

d2networking/d2netpacket/packet_player_connection_request.go

@@ -8,7 +8,7 @@ import (
 // PlayerConnectionRequestPacket contains a player ID and game state.
 // It is sent by a remote client to initiate a connection (join a game).
 type PlayerConnectionRequestPacket struct {
-	Id          string                `json:"id"`
+	ID          string                `json:"id"`
 	PlayerState *d2player.PlayerState `json:"gameState"`
 }
 
@@ -18,7 +18,7 @@ func CreatePlayerConnectionRequestPacket(id string, playerState *d2player.Player
 	return NetPacket{
 		PacketType: d2netpackettype.PlayerConnectionRequest,
 		PacketData: PlayerConnectionRequestPacket{
-			Id:          id,
+			ID:          id,
 			PlayerState: playerState,
 		},
 	}

d2networking/d2netpacket/packet_player_disconnect_request.go

@@ -8,7 +8,7 @@ import (
 // PlayerDisconnectRequestPacket contains a player ID and game state.
 // It is sent by a remote client to close the connection (leave a game).
 type PlayerDisconnectRequestPacket struct {
-	Id          string                `json:"id"`
+	ID          string                `json:"id"`
 	PlayerState *d2player.PlayerState `json:"gameState"` // TODO: remove this? It isn't used.
 }
 
@@ -18,7 +18,7 @@ func CreatePlayerDisconnectRequestPacket(id string) NetPacket {
 	return NetPacket{
 		PacketType: d2netpackettype.PlayerDisconnectionNotification,
 		PacketData: PlayerDisconnectRequestPacket{
-			Id: id,
+			ID: id,
 		},
 	}
 }

d2networking/d2netpacket/packet_update_server_info.go

@@ -3,20 +3,20 @@ package d2netpacket
 import "github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket/d2netpackettype"
 
 // UpdateServerInfoPacket contains the ID for a player and the map seed.
-// It is sent by the server to synchronise these values on the client.
+// It is sent by the server to synchronize these values on the client.
 type UpdateServerInfoPacket struct {
 	Seed     int64  `json:"seed"`
-	PlayerId string `json:"playerId"`
+	PlayerID string `json:"playerId"`
 }
 
 // CreateUpdateServerInfoPacket returns a NetPacket which declares an
 // UpdateServerInfoPacket with the given player ID and map seed.
-func CreateUpdateServerInfoPacket(seed int64, playerId string) NetPacket {
+func CreateUpdateServerInfoPacket(seed int64, playerID string) NetPacket {
 	return NetPacket{
 		PacketType: d2netpackettype.UpdateServerInfo,
 		PacketData: UpdateServerInfoPacket{
 			Seed:     seed,
-			PlayerId: playerId,
+			PlayerID: playerID,
 		},
 	}
 }

d2networking/d2server/client_connection.go

@@ -9,7 +9,7 @@ import (
 // ClientConnection is an interface for abstracting local and remote
 // clients.
 type ClientConnection interface {
-	GetUniqueId() string
+	GetUniqueID() string
 	GetConnectionType() d2clientconnectiontype.ClientConnectionType
 	SendPacketToClient(packet d2netpacket.NetPacket) error
 	GetPlayerState() *d2player.PlayerState

d2networking/d2server/connection_manager.go

@@ -9,6 +9,11 @@ import (
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket"
 )
 
+const (
+	numRetries = 3
+	second     = 1000
+)
+
 // ConnectionManager is responsible for cleanup up connections accepted by the game server.
 // As the server communicates over UDP and is stateless we need to implement some loose state
 // management via a ping/pong system. ConnectionManager also handles communication for
@@ -26,8 +31,8 @@ type ConnectionManager struct {
 // the new ConnectionManager.
 func CreateConnectionManager(gameServer *GameServer) *ConnectionManager {
 	manager := &ConnectionManager{
-		retries:    3,
+		retries:    numRetries,
-		interval:   time.Millisecond * 1000,
+		interval:   time.Millisecond * second,
 		gameServer: gameServer,
 		status:     make(map[string]int),
 	}
@@ -40,6 +45,7 @@ func CreateConnectionManager(gameServer *GameServer) *ConnectionManager {
 // Run starts up any watchers for for the connection manager
 func (c *ConnectionManager) Run() {
 	log.Print("Starting connection manager...")
+
 	for {
 		c.checkPeers()
 		time.Sleep(c.interval)
@@ -49,20 +55,24 @@ func (c *ConnectionManager) Run() {
 // checkPeers manages connection validation and cleanup for all peers.
 func (c *ConnectionManager) checkPeers() {
 	for id, connection := range c.gameServer.clientConnections {
-		if connection.GetConnectionType() != d2clientconnectiontype.Local {
+		if connection.GetConnectionType() == d2clientconnectiontype.Local {
-			if err := connection.SendPacketToClient(d2netpacket.CreatePingPacket()); err != nil {
+			continue
-				log.Printf("Cannot ping client id: %s", id)
-			}
-			c.RWMutex.Lock()
-			c.status[id] += 1
-
-			if c.status[id] >= c.retries {
-				delete(c.status, id)
-				c.Drop(id)
-			}
-
-			c.RWMutex.Unlock()
 		}
+
+		if err := connection.SendPacketToClient(d2netpacket.CreatePingPacket()); err != nil {
+			log.Printf("Cannot ping client id: %s", id)
+		}
+
+		c.RWMutex.Lock()
+
+		c.status[id]++
+
+		if c.status[id] >= c.retries {
+			delete(c.status, id)
+			c.Drop(id)
+		}
+
+		c.RWMutex.Unlock()
 	}
 }
 
@@ -84,11 +94,13 @@ func (c *ConnectionManager) Shutdown() {
 	// TODO: Currently this will never actually get called as the go routines are never signaled about the application termination.
 	// Things can be done more cleanly once we have graceful exits however we still need to account for other OS Signals
 	log.Print("Notifying clients server is shutting down...")
+
 	for _, connection := range c.gameServer.clientConnections {
 		err := connection.SendPacketToClient(d2netpacket.CreateServerClosedPacket())
 		if err != nil {
-			log.Printf("ConnectionManager: error sending ServerClosedPacket to client ID %s: %s", connection.GetUniqueId(), err)
+			log.Printf("ConnectionManager: error sending ServerClosedPacket to client ID %s: %s", connection.GetUniqueID(), err)
 		}
 	}
+
 	Stop()
 }

d2networking/d2server/d2udpclientconnection/udp_client_connection.go

@@ -5,7 +5,6 @@ import (
 	"bytes"
 	"compress/gzip"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net"
 
@@ -38,8 +37,8 @@ func CreateUDPClientConnection(udpConnection *net.UDPConn, id string, address *n
 	return result
 }
 
-// GetUniqueId returns UDPClientConnection.id
+// GetUniqueID returns UDPClientConnection.id
-func (u UDPClientConnection) GetUniqueId() string {
+func (u UDPClientConnection) GetUniqueID() string {
 	return u.id
 }
 
@@ -56,20 +55,26 @@ func (u *UDPClientConnection) SendPacketToClient(packet d2netpacket.NetPacket) e
 	if err != nil {
 		return err
 	}
+
 	var buff bytes.Buffer
+
 	buff.WriteByte(byte(packet.PacketType))
+
 	writer, _ := gzip.NewWriterLevel(&buff, gzip.BestCompression)
 
-	if written, err := writer.Write(data); err != nil {
+	if written, writeErr := writer.Write(data); writeErr != nil {
-		return err
+		return writeErr
 	} else if written == 0 {
-		return errors.New(fmt.Sprintf("RemoteClientConnection: attempted to send empty %v packet body.", packet.PacketType))
+		return fmt.Errorf("RemoteClientConnection: attempted to send empty %v packet body",
+			packet.PacketType)
 	}
-	if err = writer.Close(); err != nil {
+
-		return err
+	if writeErr := writer.Close(); writeErr != nil {
+		return writeErr
 	}
-	if _, err = u.udpConnection.WriteToUDP(buff.Bytes(), u.address); err != nil {
+
-		return err
+	if _, udpErr := u.udpConnection.WriteToUDP(buff.Bytes(), u.address); udpErr != nil {
+		return udpErr
 	}
 
 	return nil

d2networking/d2server/game_server.go

@@ -12,11 +12,9 @@ import (
 	"sync"
 	"time"
 
-	"github.com/OpenDiablo2/OpenDiablo2/d2core/d2map/d2mapgen"
-
-	"github.com/OpenDiablo2/OpenDiablo2/d2core/d2map/d2mapengine"
-
 	"github.com/OpenDiablo2/OpenDiablo2/d2common/d2enum"
+	"github.com/OpenDiablo2/OpenDiablo2/d2core/d2map/d2mapengine"
+	"github.com/OpenDiablo2/OpenDiablo2/d2core/d2map/d2mapgen"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2netpacket/d2netpackettype"
 	"github.com/OpenDiablo2/OpenDiablo2/d2networking/d2server/d2udpclientconnection"
@@ -24,6 +22,12 @@ import (
 	"github.com/robertkrimen/otto"
 )
 
+const (
+	udpBufferSize      = 4096
+	subtilesPerTile    = 5
+	middleOfTileOffset = 3
+)
+
 // GameServer owns the authoritative copy of the map and entities
 // It accepts incoming connections from local (host) and remote
 // clients.
@@ -38,6 +42,7 @@ type GameServer struct {
 	running           bool
 }
 
+//nolint:gochecknoglobals // currently singleton by design
 var singletonServer *GameServer
 
 // Create constructs a new GameServer and assigns it as a singleton. It
@@ -47,6 +52,7 @@ var singletonServer *GameServer
 // packets.
 func Create(openNetworkServer bool) {
 	log.Print("Creating GameServer")
+
 	if singletonServer != nil {
 		return
 	}
@@ -62,7 +68,10 @@ func Create(openNetworkServer bool) {
 
 	mapEngine := d2mapengine.CreateMapEngine()
 	mapEngine.SetSeed(singletonServer.seed)
-	mapEngine.ResetMap(d2enum.RegionAct1Town, 100, 100) // TODO: Mapgen - Needs levels.txt stuff
+
+	// TODO: Mapgen - Needs levels.txt stuff
+	mapEngine.ResetMap(d2enum.RegionAct1Town, 100, 100)
+
 	d2mapgen.GenerateAct1Overworld(mapEngine)
 	singletonServer.mapEngines = append(singletonServer.mapEngines, mapEngine)
 
@@ -89,7 +98,9 @@ func createNetworkServer() {
 	if err != nil {
 		panic(err)
 	}
-	err = singletonServer.udpConnection.SetReadBuffer(4096)
+
+	err = singletonServer.udpConnection.SetReadBuffer(udpBufferSize)
+
 	if err != nil {
 		log.Print("GameServer: error setting UDP read buffer:", err)
 	}
@@ -99,95 +110,146 @@ func createNetworkServer() {
 // connection.
 func runNetworkServer() {
 	buffer := make([]byte, 4096)
+
 	for singletonServer.running {
-		_, addr, err := singletonServer.udpConnection.ReadFromUDP(buffer)
+		_, addr, udpReadErr := singletonServer.udpConnection.ReadFromUDP(buffer)
-		if err != nil {
+		if udpReadErr != nil {
-			fmt.Printf("Socket error: %s\n", err)
+			fmt.Printf("Socket error: %s\n", udpReadErr)
 			continue
 		}
+
 		buff := bytes.NewBuffer(buffer)
-		packetTypeId, err := buff.ReadByte()
+
-		packetType := d2netpackettype.NetPacketType(packetTypeId)
+		packetTypeID, _ := buff.ReadByte()
-		reader, err := gzip.NewReader(buff)
+		packetType := d2netpackettype.NetPacketType(packetTypeID)
+
+		reader, _ := gzip.NewReader(buff)
 		sb := new(strings.Builder)
 
 		// This will throw errors where packets are not compressed. This doesn't
 		// break anything, so the gzip.ErrHeader error, is currently ignored to
 		// avoid noisy logging.
-		written, err := io.Copy(sb, reader)
+		written, copyErr := io.Copy(sb, reader)
-		if err != nil && err != gzip.ErrHeader {
-			log.Printf("GameServer: error copying bytes from %v packet: %s", packetType, err)
 
+		if copyErr != nil && copyErr != gzip.ErrHeader {
+			log.Printf("GameServer: error copying bytes from %v packet: %s", packetType, copyErr)
 		}
+
 		if written == 0 {
 			log.Printf("GameServer: empty packet %v packet received", packetType)
 			continue
 		}
 
 		stringData := sb.String()
+
 		switch packetType {
 		case d2netpackettype.PlayerConnectionRequest:
-			packetData := d2netpacket.PlayerConnectionRequestPacket{}
+			if err := handlePlayerConnectionRequest(addr, stringData); err != nil {
-			err := json.Unmarshal([]byte(stringData), &packetData)
+				log.Printf("GameServer error: %v", err)
-			if err != nil {
-				log.Printf("GameServer: error unmarshalling packet of type %T: %s", packetData, err)
-				continue
 			}
-			clientConnection := d2udpclientconnection.CreateUDPClientConnection(singletonServer.udpConnection, packetData.Id, addr)
-			clientConnection.SetPlayerState(packetData.PlayerState)
-			OnClientConnected(clientConnection)
 		case d2netpackettype.MovePlayer:
-			packetData := d2netpacket.MovePlayerPacket{}
+			if err := handleMovePlayer(packetType, stringData); err != nil {
-			err := json.Unmarshal([]byte(stringData), &packetData)
+				log.Printf("GameServer error: %v", err)
-			if err != nil {
-				log.Printf("GameServer: error unmarshalling %T: %s", packetData, err)
-				continue
-			}
-			netPacket := d2netpacket.NetPacket{
-				PacketType: packetType,
-				PacketData: packetData,
-			}
-
-			for _, player := range singletonServer.clientConnections {
-				err = player.SendPacketToClient(netPacket)
-				if err != nil {
-					log.Printf("GameServer: error sending %T to client %s: %s", packetData, player.GetUniqueId(), err)
-				}
 			}
 		case d2netpackettype.Pong:
-			packetData := d2netpacket.PlayerConnectionRequestPacket{}
+			if err := handlePingPong(stringData); err != nil {
-			err := json.Unmarshal([]byte(stringData), &packetData)
+				log.Printf("GameServer error: %v", err)
-			if err != nil {
-				log.Printf("GameServer: error unmarshalling packet of type %T: %s", packetData, err)
-				continue
 			}
-			singletonServer.manager.Recv(packetData.Id)
 		case d2netpackettype.ServerClosed:
 			singletonServer.manager.Shutdown()
 		case d2netpackettype.PlayerDisconnectionNotification:
-			var packet d2netpacket.PlayerDisconnectRequestPacket
+			if err := handlePlayerDisconnectNotification(stringData); err != nil {
-			err := json.Unmarshal([]byte(stringData), &packet)
+				log.Printf("GameServer error: %v", err)
-			if err != nil {
-				log.Printf("GameServer: error unmarshalling packet of type %T: %s", packet, err)
-				continue
 			}
-			log.Printf("Received disconnect: %s", packet.Id)
 		}
 	}
 }
 
+func handlePlayerConnectionRequest(addr *net.UDPAddr, stringData string) error {
+	packetData := d2netpacket.PlayerConnectionRequestPacket{}
+	err := json.Unmarshal([]byte(stringData), &packetData)
+
+	if err != nil {
+		log.Printf("GameServer: error unmarshalling packet of type %T: %s", packetData, err)
+		return err
+	}
+
+	clientConnection := d2udpclientconnection.CreateUDPClientConnection(singletonServer.udpConnection, packetData.ID, addr)
+
+	clientConnection.SetPlayerState(packetData.PlayerState)
+	OnClientConnected(clientConnection)
+
+	return nil
+}
+
+func handleMovePlayer(packetType d2netpackettype.NetPacketType, stringData string) error {
+	packetData := d2netpacket.MovePlayerPacket{}
+	err := json.Unmarshal([]byte(stringData), &packetData)
+
+	if err != nil {
+		log.Printf("GameServer: error unmarshalling %T: %s", packetData, err)
+		return err
+	}
+
+	netPacket := d2netpacket.NetPacket{
+		PacketType: packetType,
+		PacketData: packetData,
+	}
+
+	for _, player := range singletonServer.clientConnections {
+		err = player.SendPacketToClient(netPacket)
+		if err != nil {
+			log.Printf("GameServer: error sending %T to client %s: %s", packetData, player.GetUniqueID(), err)
+		}
+	}
+
+	return nil
+}
+
+func handlePingPong(stringData string) error {
+	packetData := d2netpacket.PlayerConnectionRequestPacket{}
+	err := json.Unmarshal([]byte(stringData), &packetData)
+
+	if err != nil {
+		log.Printf("GameServer: error unmarshalling packet of type %T: %s", packetData, err)
+		return err
+	}
+
+	singletonServer.manager.Recv(packetData.ID)
+
+	return nil
+}
+
+func handlePlayerDisconnectNotification(stringData string) error {
+	var packet d2netpacket.PlayerDisconnectRequestPacket
+	err := json.Unmarshal([]byte(stringData), &packet)
+
+	if err != nil {
+		log.Printf("GameServer: error unmarshalling packet of type %T: %s", packet, err)
+		return err
+	}
+
+	log.Printf("Received disconnect: %s", packet.ID)
+
+	return nil
+}
+
 // Run sets GameServer.running to true and call runNetworkServer
 // in a goroutine.
 func Run() {
 	log.Print("Starting GameServer")
+
 	singletonServer.running = true
 	_, err := singletonServer.scriptEngine.RunScript("scripts/server/server.js")
+
 	if err != nil {
 		log.Printf("GameServer: error initializing debug script: %s", err)
 	}
+
 	if singletonServer.udpConnection != nil {
 		go runNetworkServer()
 	}
+
 	log.Print("Network server has been started")
 }
 
@@ -195,7 +257,9 @@ func Run() {
 // GameServer's UDP connection.
 func Stop() {
 	log.Print("Stopping GameServer")
+
 	singletonServer.running = false
+
 	if singletonServer.udpConnection != nil {
 		err := singletonServer.udpConnection.Close()
 		if err != nil {
@@ -209,7 +273,9 @@ func Destroy() {
 	if singletonServer == nil {
 		return
 	}
+
 	log.Print("Destroying GameServer")
+
 	Stop()
 }
 
@@ -229,44 +295,62 @@ func OnClientConnected(client ClientConnection) {
 	clientPlayerState.Y = sy
 	// --------------------------------------------------------------------
 
-	log.Printf("Client connected with an id of %s", client.GetUniqueId())
+	log.Printf("Client connected with an id of %s", client.GetUniqueID())
-	singletonServer.clientConnections[client.GetUniqueId()] = client
+	singletonServer.clientConnections[client.GetUniqueID()] = client
-	err := client.SendPacketToClient(d2netpacket.CreateUpdateServerInfoPacket(singletonServer.seed, client.GetUniqueId()))
+	err := client.SendPacketToClient(d2netpacket.CreateUpdateServerInfoPacket(singletonServer.seed, client.GetUniqueID()))
+
 	if err != nil {
-		log.Printf("GameServer: error sending UpdateServerInfoPacket to client %s: %s", client.GetUniqueId(), err)
+		log.Printf("GameServer: error sending UpdateServerInfoPacket to client %s: %s", client.GetUniqueID(), err)
 	}
+
 	err = client.SendPacketToClient(d2netpacket.CreateGenerateMapPacket(d2enum.RegionAct1Town))
+
 	if err != nil {
-		log.Printf("GameServer: error sending GenerateMapPacket to client %s: %s", client.GetUniqueId(), err)
+		log.Printf("GameServer: error sending GenerateMapPacket to client %s: %s", client.GetUniqueID(), err)
 	}
 
 	playerState := client.GetPlayerState()
-	createPlayerPacket := d2netpacket.CreateAddPlayerPacket(client.GetUniqueId(), playerState.HeroName, int(sx*5)+3, int(sy*5)+3,
+
-		playerState.HeroType, *playerState.Stats, playerState.Equipment)
+	// these are in subtiles
+	playerX := int(sx*subtilesPerTile) + middleOfTileOffset
+	playerY := int(sy*subtilesPerTile) + middleOfTileOffset
+
+	createPlayerPacket := d2netpacket.CreateAddPlayerPacket(client.GetUniqueID(),
+		playerState.HeroName, playerX, playerY,
+		playerState.HeroType, playerState.Stats, playerState.Equipment)
+
 	for _, connection := range singletonServer.clientConnections {
 		err := connection.SendPacketToClient(createPlayerPacket)
 		if err != nil {
-			log.Printf("GameServer: error sending %T to client %s: %s", createPlayerPacket, connection.GetUniqueId(), err)
+			log.Printf("GameServer: error sending %T to client %s: %s", createPlayerPacket, connection.GetUniqueID(), err)
 		}
-		if connection.GetUniqueId() == client.GetUniqueId() {
+
+		if connection.GetUniqueID() == client.GetUniqueID() {
 			continue
 		}
 
 		conPlayerState := connection.GetPlayerState()
-		err = client.SendPacketToClient(d2netpacket.CreateAddPlayerPacket(connection.GetUniqueId(), conPlayerState.HeroName,
+		playerX := int(conPlayerState.X*subtilesPerTile) + middleOfTileOffset
-			int(conPlayerState.X*5)+3, int(conPlayerState.Y*5)+3, conPlayerState.HeroType, *conPlayerState.Stats, conPlayerState.Equipment))
+		playerY := int(conPlayerState.Y*subtilesPerTile) + middleOfTileOffset
+		err = client.SendPacketToClient(d2netpacket.CreateAddPlayerPacket(
+			connection.GetUniqueID(),
+			conPlayerState.HeroName,
+			playerX, playerY,
+			conPlayerState.HeroType,
+			conPlayerState.Stats, conPlayerState.Equipment),
+		)
+
 		if err != nil {
-			log.Printf("GameServer: error sending CreateAddPlayerPacket to client %s: %s", connection.GetUniqueId(), err)
+			log.Printf("GameServer: error sending CreateAddPlayerPacket to client %s: %s", connection.GetUniqueID(), err)
 		}
 	}
-
 }
 
 // OnClientDisconnected removes the given client from the list
 // of client connections.
 func OnClientDisconnected(client ClientConnection) {
-	log.Printf("Client disconnected with an id of %s", client.GetUniqueId())
+	log.Printf("Client disconnected with an id of %s", client.GetUniqueID())
-	delete(singletonServer.clientConnections, client.GetUniqueId())
+	delete(singletonServer.clientConnections, client.GetUniqueID())
 }
 
 // OnPacketReceived is called by the local client to 'send' a packet to the server.
@@ -276,23 +360,24 @@ func OnPacketReceived(client ClientConnection, packet d2netpacket.NetPacket) err
 		// TODO: This needs to be verified on the server (here) before sending to other clients....
 		// TODO: Hacky, this should be updated in realtime ----------------
 		// TODO: Verify player id
-		playerState := singletonServer.clientConnections[client.GetUniqueId()].GetPlayerState()
+		playerState := singletonServer.clientConnections[client.GetUniqueID()].GetPlayerState()
 		playerState.X = packet.PacketData.(d2netpacket.MovePlayerPacket).DestX
 		playerState.Y = packet.PacketData.(d2netpacket.MovePlayerPacket).DestY
 		// ----------------------------------------------------------------
 		for _, player := range singletonServer.clientConnections {
 			err := player.SendPacketToClient(packet)
 			if err != nil {
-				log.Printf("GameServer: error sending %T to client %s: %s", packet, player.GetUniqueId(), err)
+				log.Printf("GameServer: error sending %T to client %s: %s", packet, player.GetUniqueID(), err)
 			}
 		}
 	case d2netpackettype.CastSkill:
 		for _, player := range singletonServer.clientConnections {
 			err := player.SendPacketToClient(packet)
 			if err != nil {
-				log.Printf("GameServer: error sending %T to client %s: %s", packet, player.GetUniqueId(), err)
+				log.Printf("GameServer: error sending %T to client %s: %s", packet, player.GetUniqueID(), err)
 			}
 		}
 	}
+
 	return nil
 }