Removed all TODO's in project (#831)

* removed the rest of the magic number errors from d2game

* hotfix for bug i added in map engine test

* removed all TODO's in project, made issues on github for each one

d2common/d2enum/npc_action_type.go

@@ -4,7 +4,7 @@ package d2enum
 type NPCActionType int
 
 // NPCAction types
-// TODO: Figure out what 1-3 are for
+// https://github.com/OpenDiablo2/OpenDiablo2/issues/811
 const (
 	NPCActionInvalid NPCActionType = iota
 	NPCAction1

d2common/d2fileformats/d2dcc/dcc_direction.go

@@ -192,7 +192,7 @@ func (v *DCCDirection) generateFrames(pcd *d2datautils.BitMuncher) {
 			if (pbe.Frame != frameIndex) || (pbe.FrameCellIndex != c) {
 				// This buffer cell has an EqualCell bit set to 1, so copy the frame cell or clear it
 				if (cell.Width != bufferCell.LastWidth) || (cell.Height != bufferCell.LastHeight) {
-					// Different sizes TODO: Clear the pixels of the frame cell
+					// Different sizes
 					for y := 0; y < cell.Height; y++ {
 						for x := 0; x < cell.Width; x++ {
 							v.PixelData[x+cell.XOffset+((y+cell.YOffset)*v.Box.Width)] = 0

d2common/d2fileformats/d2mpq/mpq.go

@@ -192,7 +192,7 @@ func (v *MPQ) loadHashTable() error {
 		v.hashEntryMap.Insert(&HashTableEntry{
 			NamePartA: hashData[i*4],
 			NamePartB: hashData[(i*4)+1],
-			//nolint:godox    // TODO: Verify that we're grabbing the right high/lo word for the vars below
+			// https://github.com/OpenDiablo2/OpenDiablo2/issues/812
 			Locale:     uint16(hashData[(i*4)+2] >> 16),    //nolint:gomnd binary data
 			Platform:   uint16(hashData[(i*4)+2] & 0xFFFF), //nolint:gomnd binary data
 			BlockIndex: hashData[(i*4)+3],

d2common/d2interface/animation.go

@@ -34,7 +34,6 @@ type Animation interface {
 	SetPlayLoop(loop bool)
 	SetPlaySpeed(playSpeed float64)
 	SetPlayLength(playLength float64)
-	SetPlayLengthMs(playLengthMs int)
 	SetColorMod(colorMod color.Color)
 	GetPlayedCount() int
 	ResetPlayedCount()

d2core/d2asset/animation.go

@@ -51,7 +51,7 @@ type Animation struct {
 	lastFrameTime    float64
 	playedCount      int
 	playMode         playMode
-	playLength       float64
+	playLength       float64 // https://github.com/OpenDiablo2/OpenDiablo2/issues/813
 	subStartingFrame int
 	subEndingFrame   int
 	originAtBottom   bool
@@ -367,19 +367,11 @@ func (a *Animation) SetPlaySpeed(playSpeed float64) {
 
 // SetPlayLength sets the Animation's play length in seconds
 func (a *Animation) SetPlayLength(playLength float64) {
-	// TODO refactor to use time.Duration instead of float64
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/813
 	a.playLength = playLength
 	a.lastFrameTime = 0
 }
 
-// SetPlayLengthMs sets the Animation's play length in milliseconds
-func (a *Animation) SetPlayLengthMs(playLengthMs int) {
-	// TODO remove this method
-	const millisecondsPerSecond = 1000.0
-
-	a.SetPlayLength(float64(playLengthMs) / millisecondsPerSecond)
-}
-
 // SetColorMod sets the Animation's color mod
 func (a *Animation) SetColorMod(colorMod color.Color) {
 	a.colorMod = colorMod

d2core/d2hero/hero_state_factory.go

@@ -131,7 +131,6 @@ func (f *HeroStateFactory) CreateHeroSkillsState(classStats *d2records.CharStats
 		}
 	}
 
-
 	skillRecord, err := f.CreateHeroSkill(1, "Attack")
 	if err != nil {
 		return nil, err
@@ -158,7 +157,7 @@ func (f *HeroStateFactory) CreateHeroSkill(points int, name string) (*HeroSkill,
 		SkillPoints:            points,
 		SkillRecord:            skillRecord,
 		SkillDescriptionRecord: skillDescRecord,
-		shallow: &shallowHeroSkill{SkillID: skillRecord.ID, SkillPoints: points},
+		shallow:                &shallowHeroSkill{SkillID: skillRecord.ID, SkillPoints: points},
 	}
 
 	return result, nil
@@ -200,7 +199,6 @@ func (f *HeroStateFactory) LoadHeroState(filePath string) *HeroState {
 	for idx := range result.Skills {
 		hs := result.Skills[idx]
 
-		// TODO: figure out why this can be nil
 		if hs == nil {
 			continue
 		}

d2core/d2hero/hero_stats_state.go

@@ -48,14 +48,14 @@ func (f *HeroStateFactory) CreateHeroStatsState(heroClass d2enum.Hero, classStat
 		MaxHealth:  classStats.InitVit * classStats.LifePerVit,
 		MaxMana:    classStats.InitEne * classStats.ManaPerEne,
 		MaxStamina: classStats.InitStamina,
-		// TODO: chance to hit, defense rating
+		// https://github.com/OpenDiablo2/OpenDiablo2/issues/814
 	}
 
 	result.Mana = result.MaxMana
 	result.Health = result.MaxHealth
 	result.Stamina = float64(result.MaxStamina)
 
-	// TODO: For demonstration purposes (hp, mana, exp, & character stats panel gets updated depending on stats)
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/815
 	result.Health = 50
 	result.Mana = 30
 	result.Experience = 166

d2core/d2item/diablo2item/item.go

@@ -441,7 +441,8 @@ func (i *Item) generateProperties(pool PropertyPool) {
 		if generated := i.generateSetItemProperties(); generated != nil {
 			props = generated
 		}
-	case PropertyPoolSet: // todo set bonus handling, needs player/equipment context
+	case PropertyPoolSet:
+		// https://github.com/OpenDiablo2/OpenDiablo2/issues/817
 	}
 
 	if props == nil {

d2core/d2item/diablo2item/item_factory.go

@@ -388,7 +388,7 @@ func (f *ItemFactory) resolveDynamicTreasureCode(code string) []*d2records.ItemC
 	stringComponent := getStringComponent(code)
 
 	if stringComponent == goldItemCodeWithMult {
-		// todo need to do something with the numeric component (the gold multiplier)
+		// need to do something with the numeric component (the gold multiplier)
 		stringComponent = goldItemCode
 	}
 

d2core/d2item/diablo2item/item_property.go

@@ -354,7 +354,7 @@ func (p *Property) fnClassSkills(
 
 // fnStateApplyToTarget property applied to character or target monster ???
 func (p *Property) fnStateApplyToTarget(iscRecord *d2records.ItemStatCostRecord) d2stats.Stat {
-	// todo need to implement states
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/818
 	return nil
 }
 

d2core/d2map/d2mapentity/cast_overlay.go

@@ -33,7 +33,7 @@ func (co *CastOverlay) GetVelocity() d2vector.Vector {
 // SetRadians adjusts the entity target based on it's range, rotating it's
 // current destination by the value of angle in radians.
 func (co *CastOverlay) SetRadians(angle float64, done func()) {
-	rads := float64(co.record.Height2) // TODO:
+	rads := float64(co.record.Height2)
 
 	x := co.Position.X() + (rads * math.Cos(angle))
 	y := co.Position.Y() + (rads * math.Sin(angle))

d2core/d2map/d2mapentity/missile.go

@@ -43,7 +43,7 @@ func (m *Missile) SetRadians(angle float64, done func()) {
 // Advance is called once per frame and processes a
 // single game tick.
 func (m *Missile) Advance(tickTime float64) {
-	// TODO: collision detection
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/819
 	m.Step(tickTime)
 	m.AnimatedEntity.Advance(tickTime)
 }

d2core/d2map/d2mapentity/player.go

@@ -254,7 +254,7 @@ func (p *Player) GetVelocity() d2vector.Vector {
 // GetSize returns the current frame size
 func (p *Player) GetSize() (width, height int) {
 	width, height = p.composite.GetSize()
-	// todo: we need to get full size of composite animations, currently only gets legs
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/820
 	height = (height * 2) - (height / 2)
 
 	return width, height

d2core/d2map/d2maprenderer/renderer.go

@@ -124,7 +124,7 @@ func (mr *MapRenderer) SetMapEngine(mapEngine *d2mapengine.MapEngine) {
 //
 // Pass 4: Roof tiles.
 func (mr *MapRenderer) Render(target d2interface.Surface) {
-	// TODO:(temp hack) should not render before the map has been fully generated -
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/789
 	// Prevents concurrent map read & write exceptions that otherwise occur when we join a TCP game
 	// as a remote client, due to rendering before we have handled the GenerateMapPacket.
 	if mr.mapEngine.IsLoading {
@@ -208,7 +208,8 @@ func (mr *MapRenderer) renderPass2(target d2interface.Surface, startX, startY, e
 
 			tileEnt := make([]d2interface.MapEntity, 0)
 
-			// TODO: Do not loop over every entity every frame
+			// need to add render culling
+			// https://github.com/OpenDiablo2/OpenDiablo2/issues/821
 			for _, mapEntity := range mr.mapEngine.Entities() {
 				pos := mapEntity.GetPosition()
 				vec := pos.World()
@@ -255,7 +256,8 @@ func (mr *MapRenderer) renderPass3(target d2interface.Surface, startX, startY, e
 
 			tileEnt := make([]d2interface.MapEntity, 0)
 
-			// TODO: Do not loop over every entity every frame
+			// need to add render culling
+			// https://github.com/OpenDiablo2/OpenDiablo2/issues/821
 			for _, mapEntity := range mr.mapEngine.Entities() {
 				pos := mapEntity.GetPosition()
 				vec := pos.World()

d2core/d2records/skill_description_loader.go

@@ -13,7 +13,7 @@ func skillDescriptionLoader(r *RecordManager, d *d2txt.DataDictionary) error {
 	records := make(map[string]*SkillDescriptionRecord)
 
 	parser := d2parser.New()
-	parser.SetCurrentReference("skill", "TODO: connect skill with description!") //nolint:godox // TODO: Connect skill with description.
+	parser.SetCurrentReference("skill", "TODO: connect skill with description!")
 
 	for d.Next() {
 		record := &SkillDescriptionRecord{

d2core/d2render/ebiten/ebiten_surface.go

@@ -192,7 +192,7 @@ func (s *ebitenSurface) handleStateEffect(opts *ebiten.DrawImageOptions) {
 		opts.ColorM.Translate(0, 0, 0, -transparency75)
 	case d2enum.DrawEffectModulate:
 		opts.CompositeMode = ebiten.CompositeModeLighter
-	// TODO: idk what to do when ebiten doesn't exactly match, pick closest?
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/822
 	case d2enum.DrawEffectBurn:
 	case d2enum.DrawEffectNormal:
 	case d2enum.DrawEffectMod2XTrans:

d2core/d2ui/label.go

@@ -173,7 +173,7 @@ func (v *Label) getAlignOffset(textWidth int) int {
 }
 
 func getColor(token ColorToken) color.Color {
-	// todo this should really come from the PL2 files
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/823
 	colors := map[ColorToken]color.Color{
 		ColorTokenGrey:   d2util.Color(colorGrey100Alpha),
 		ColorTokenWhite:  d2util.Color(colorWhite100Alpha),

d2core/d2ui/sprite.go

@@ -186,11 +186,6 @@ func (s *Sprite) SetPlayLength(playLength float64) {
 	s.animation.SetPlayLength(playLength)
 }
 
-// SetPlayLengthMs sets the play length of the sprite animation in milliseconds
-func (s *Sprite) SetPlayLengthMs(playLengthMs int) {
-	s.animation.SetPlayLengthMs(playLengthMs)
-}
-
 // SetColorMod sets the color modifier
 func (s *Sprite) SetColorMod(c color.Color) {
 	s.animation.SetColorMod(c)

d2core/d2ui/ui_manager.go

@@ -17,9 +17,9 @@ type UIManager struct {
 	inputManager  d2interface.InputManager
 	audio         d2interface.AudioProvider
 	widgets       []Widget
-	cursorButtons CursorButton // TODO (carrelld) convert dependent code and remove
+	cursorButtons CursorButton
-	CursorX       int          // TODO (carrelld) convert dependent code and remove
+	CursorX       int
-	CursorY       int          // TODO (carrelld) convert dependent code and remove
+	CursorY       int
 	pressedWidget Widget
 	clickSfx      d2interface.SoundEffect
 }

d2game/d2gamescreen/select_hero_class.go

@@ -776,7 +776,7 @@ func (v *SelectHeroClass) loadSprite(animationPath string, position image.Point,
 	}
 
 	if playLength != 0 {
-		sprite.SetPlayLengthMs(playLength)
+		sprite.SetPlayLength(float64(playLength) / 1000.0)
 	}
 
 	sprite.SetPosition(position.X, position.Y)

d2networking/d2client/d2localclient/local_client_connection.go

@@ -92,7 +92,6 @@ func (l *LocalClientConnection) Close() error {
 
 // SendPacketToServer calls d2server.OnPacketReceived with the given packet.
 func (l *LocalClientConnection) SendPacketToServer(packet d2netpacket.NetPacket) error {
-	// TODO: This is going to blow up if the server has ceased to be.
 	return d2server.OnPacketReceived(l, packet)
 }
 

d2networking/d2client/d2remoteclient/remote_client_connection.go

@@ -50,16 +50,13 @@ func (r *RemoteClientConnection) Open(connectionString, saveFilePath string) err
 		connectionString += ":6669"
 	}
 
-	// TODO: Connect to the server
 	tcpAddress, err := net.ResolveTCPAddr("tcp", connectionString)
 
-	// TODO: Show connection error screen if connection fails
 	if err != nil {
 		return err
 	}
 
 	r.tcpConnection, err = net.DialTCP("tcp", nil, tcpAddress)
-	// TODO: Show connection error screen if connection fails
 	if err != nil {
 		return err
 	}

d2networking/d2netpacket/packet_move_player.go

@@ -9,7 +9,7 @@ import (
 
 // MovePlayerPacket contains a movement command for a specific player entity.
 // It is sent by the server to move a player entity on a client.
-// TODO: Need to handle being on different maps
+// https://github.com/OpenDiablo2/OpenDiablo2/issues/825
 type MovePlayerPacket struct {
 	PlayerID string  `json:"playerId"`
 	StartX   float64 `json:"startX"`

d2networking/d2netpacket/packet_player_cast.go

@@ -10,7 +10,6 @@ import (
 // CastPacket contains a cast command for an entity. It is sent by the server
 // and instructs the client to trigger the use of the given skill on the given
 // entity.
-// TODO: Need to handle being on different maps
 type CastPacket struct {
 	SourceEntityID string  `json:"sourceEntityId"`
 	SkillID        int     `json:"skillId"`
@@ -27,7 +26,7 @@ func CreateCastPacket(entityID string, skillID int, targetX, targetY float64) Ne
 		SkillID:        skillID,
 		TargetX:        targetX,
 		TargetY:        targetY,
-		TargetEntityID: "", // TODO implement targeting entities
+		TargetEntityID: "", // https://github.com/OpenDiablo2/OpenDiablo2/issues/826
 	}
 
 	b, err := json.Marshal(castPacket)

d2networking/d2netpacket/packet_player_disconnect_request.go

@@ -13,7 +13,7 @@ import (
 // It is sent by a remote client to close the connection (leave a game).
 type PlayerDisconnectRequestPacket struct {
 	ID          string            `json:"id"`
-	PlayerState *d2hero.HeroState `json:"gameState"` // TODO: remove this? It isn't used.
+	PlayerState *d2hero.HeroState `json:"gameState"`
 }
 
 // CreatePlayerDisconnectRequestPacket returns a NetPacket which defines a

d2networking/d2server/game_server.go

@@ -52,6 +52,7 @@ type GameServer struct {
 	packetManagerChan chan []byte
 }
 
+// https://github.com/OpenDiablo2/OpenDiablo2/issues/824
 //nolint:gochecknoglobals // currently singleton by design
 var singletonServer *GameServer
 
@@ -82,8 +83,7 @@ func NewGameServer(asset *d2asset.AssetManager, networkServer bool,
 		seed:              time.Now().UnixNano(),
 	}
 
-	// TODO: In order to support dedicated mode we need to load the levels txt and files. Revisit this once this we can
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/827
-	//   load files independent of the app.
 	mapEngine := d2mapengine.CreateMapEngine(asset)
 	mapEngine.SetSeed(gameServer.seed)
 	mapEngine.ResetMap(d2enum.RegionAct1Town, 100, 100)
@@ -105,8 +105,7 @@ func NewGameServer(asset *d2asset.AssetManager, networkServer bool,
 		return val
 	})
 
-	// TODO: Temporary hack to work around local connections. Possible that we can move away from the singleton pattern here
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/824
-	// 		but for now this will work.
 	singletonServer = gameServer
 
 	return gameServer, nil
@@ -245,11 +244,10 @@ func (g *GameServer) handleConnection(conn net.Conn) {
 				log.Printf("Closing connection with %s: did not receive new player connection request...\n", conn.RemoteAddr().String())
 			}
 
-			// TODO: I do not think this error check actually works. Need to retrofit with Errors.Is().
 			if err := g.registerConnection(packet.PacketData, conn); err != nil {
 				switch err {
 				case errServerFull: // Server is currently full and not accepting new connections.
-					// TODO: Need to create a new Server Full packet to return to clients.
+					// https://github.com/OpenDiablo2/OpenDiablo2/issues/828
 					log.Println(err)
 					return
 				case errPlayerAlreadyExists: // Player is already registered and did not disconnection correctly.
@@ -301,7 +299,8 @@ func (g *GameServer) registerConnection(b []byte, conn net.Conn) error {
 	g.connections[client.GetUniqueID()] = client
 
 	// Temporary position hack --------------------------------------------
-	sx, sy := g.mapEngines[0].GetStartPosition() // TODO: Another temporary hack
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/829
+	sx, sy := g.mapEngines[0].GetStartPosition()
 	clientPlayerState := client.GetPlayerState()
 	clientPlayerState.X = sx
 	clientPlayerState.Y = sy
@@ -325,7 +324,8 @@ func (g *GameServer) registerConnection(b []byte, conn net.Conn) error {
 // For more information, see d2networking.d2netpacket.
 func OnClientConnected(client ClientConnection) {
 	// Temporary position hack --------------------------------------------
-	sx, sy := singletonServer.mapEngines[0].GetStartPosition() // TODO: Another temporary hack
+	// https://github.com/OpenDiablo2/OpenDiablo2/issues/829
+	sx, sy := singletonServer.mapEngines[0].GetStartPosition()
 	clientPlayerState := client.GetPlayerState()
 	clientPlayerState.X = sx
 	clientPlayerState.Y = sy
@@ -408,15 +408,17 @@ func OnClientDisconnected(client ClientConnection) {
 
 // OnPacketReceived is called by the local client to 'send' a packet to the server.
 func OnPacketReceived(client ClientConnection, packet d2netpacket.NetPacket) error {
+	if singletonServer == nil {
+		return errors.New("singleton server is nil")
+	}
+
 	switch packet.PacketType {
 	case d2netpackettype.MovePlayer:
 		movePacket, err := d2netpacket.UnmarshalMovePlayer(packet.PacketData)
 		if err != nil {
 			return err
 		}
-		// TODO: This needs to be verified on the server (here) before sending to other clients....
+		// https://github.com/OpenDiablo2/OpenDiablo2/issues/830
-		// TODO: Hacky, this should be updated in realtime ----------------
-		// TODO: Verify player id
 		playerState := singletonServer.connections[client.GetUniqueID()].GetPlayerState()
 		playerState.X = movePacket.DestX
 		playerState.Y = movePacket.DestY