feat: adicionado classes iniciais de jogo que utilizara a classe mapsc… #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…anner para escanear o mapa atual previamente construido dentro da godot
Guia do Revisor por SourceryEste pull request introduz as classes de jogo iniciais e a lógica de varredura de mapa. Ele configura a infraestrutura necessária para varrer um mapa usando TileMaps no Godot e armazenar os dados varridos em uma grade de tiles. Diagrama de sequência para o processo de varredura de mapasequenceDiagram
participant Game
participant MapScanner
participant TileMapManager
participant TilesManager
Game->>TilesManager: SetUpTiles(width, height)
Game->>MapScanner: Scan(width, height)
activate MapScanner
loop Para cada posição de tile
MapScanner->>TileMapManager: GetCellTileData(position)
MapScanner->>TilesManager: TryGetTileAt(x, y)
MapScanner->>TilesManager: Atualizar propriedades do tile
end
MapScanner-->>Game: Sinal MapScanned
deactivate MapScanner
Game->>Game: OnMapScanned()
Diagrama de classes para o novo sistema de varredura de mapaclassDiagram
class Game {
-TilesManager _tilesManager
+MapSettingsResource MapSettings
-bool MapIsLoading
+_Ready()
+OnMapScanned()
+OnMapGenerated()
}
class MapScanner {
-TileMapManager _tileMapManager
-TilesManager _tilesManager
+Scan(int width, int height)
}
class TileMapManager {
+TileMapLayer GroundTileMap
+TileMapLayer SpawnTileMap
+_Ready()
}
class DRTileData {
+int X
+int Y
+bool IsWalkable
+bool IsSpawnPoint
}
class MapSettingsResource {
+int Width
+int Height
}
Game --> MapSettingsResource
Game --> MapScanner
MapScanner --> TileMapManager
MapScanner --> TilesManager
TilesManager --> DRTileData
Mudanças em Nível de Arquivo
Dicas e comandosInteragindo com o Sourcery
Personalizando sua ExperiênciaAcesse seu painel para:
Obtendo Ajuda
Original review guide in EnglishReviewer's Guide by SourceryThis pull request introduces the initial game classes and map scanning logic. It sets up the necessary infrastructure for scanning a map using TileMaps in Godot and storing the scanned data in a grid of tiles. Sequence diagram for map scanning processsequenceDiagram
participant Game
participant MapScanner
participant TileMapManager
participant TilesManager
Game->>TilesManager: SetUpTiles(width, height)
Game->>MapScanner: Scan(width, height)
activate MapScanner
loop For each tile position
MapScanner->>TileMapManager: GetCellTileData(position)
MapScanner->>TilesManager: TryGetTileAt(x, y)
MapScanner->>TilesManager: Update tile properties
end
MapScanner-->>Game: MapScanned signal
deactivate MapScanner
Game->>Game: OnMapScanned()
Class diagram for the new map scanning systemclassDiagram
class Game {
-TilesManager _tilesManager
+MapSettingsResource MapSettings
-bool MapIsLoading
+_Ready()
+OnMapScanned()
+OnMapGenerated()
}
class MapScanner {
-TileMapManager _tileMapManager
-TilesManager _tilesManager
+Scan(int width, int height)
}
class TileMapManager {
+TileMapLayer GroundTileMap
+TileMapLayer SpawnTileMap
+_Ready()
}
class DRTileData {
+int X
+int Y
+bool IsWalkable
+bool IsSpawnPoint
}
class MapSettingsResource {
+int Width
+int Height
}
Game --> MapSettingsResource
Game --> MapScanner
MapScanner --> TileMapManager
MapScanner --> TilesManager
TilesManager --> DRTileData
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Olá @LeonardoCruzx - Revisei suas alterações - aqui estão alguns comentários:
Comentários Gerais:
- Por favor, não envie pull requests WIP (trabalho em andamento). Volte com uma implementação completa quando estiver pronto.
- A implementação está muito básica - MapScanner.Scan() e os métodos de manipulação de mapa do Game precisam de implementações reais. Por favor, esclareça a diferença entre varredura e geração de mapa.
Aqui está o que analisei durante a revisão
- 🟡 Problemas Gerais: 1 problema encontrado
- 🟢 Segurança: tudo parece bom
- 🟢 Testes: tudo parece bom
- 🟢 Complexidade: tudo parece bom
- 🟢 Documentação: tudo parece bom
Sourcery é gratuito para código aberto - se você gostou de nossas revisões, considere compartilhá-las ✨
Original comment in English
Hey @LeonardoCruzx - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please don't submit WIP (work in progress) pull requests. Come back with a complete implementation when ready.
- The implementation is too bare - MapScanner.Scan() and Game's map handling methods need actual implementations. Please clarify the difference between map scanning and generation.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
LeonardoCruzx
changed the title
LeonardoCruzx
changed the title
…e e escaner foi alterado para emitir um sinal diretamente ao script game
…s dos blocos e adiciona extensao para buscar recursivamente nos de um tipo
…Royale into feat/adicionar-escaneamento-de-mapa
|
@sourcery-ai review |
There was a problem hiding this comment.
Olá @LeonardoCruzx - Revisei suas alterações - aqui estão alguns comentários:
Comentários Gerais:
- Considere usar injeção de dependência em vez de singletons (TileMapManager.Instance, TilesManager.Instance) para melhorar a testabilidade e reduzir dependências ocultas
- O campo MapIsLoading parece não estar sendo usado - é definido, mas nunca verificado. Considere removê-lo se não for necessário ou implementar a funcionalidade pretendida
Aqui está o que analisei durante a revisão
- 🟡 Problemas Gerais: 1 problema encontrado
- 🟢 Segurança: tudo parece bom
- 🟢 Testes: tudo parece bom
- 🟢 Complexidade: tudo parece bom
- 🟢 Documentação: tudo parece bom
Sourcery é gratuito para código aberto - se você gostou de nossas revisões, considere compartilhá-las ✨
Original comment in English
Hey @LeonardoCruzx - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using dependency injection instead of singletons (TileMapManager.Instance, TilesManager.Instance) to improve testability and reduce hidden dependencies
- The MapIsLoading field appears to be unused - it's set but never checked. Consider removing it if not needed or implement the intended functionality
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| else | ||
| { | ||
| GD.PrintErr("There is already an instance of TileMapManager in the scene."); |
There was a problem hiding this comment.
sugestão (risco_de_bug): Considere lançar exceções para falhas de inicialização em vez de apenas registrar erros
Quando o gerenciador não pode ser inicializado corretamente (instância duplicada ou TileMaps necessários ausentes), o tratamento de erro atual permite que continue em um estado inválido. Isso pode levar a exceções de referência nula posteriormente.
Implementação sugerida:
public class TileMapManagerInitializationException : Exception
{
public TileMapManagerInitializationException(string message) : base(message) { }
}
public partial class TileMapManager : Node2D
{
public static TileMapManager? Instance { get; private set; } if (Instance is not null)
{
throw new TileMapManagerInitializationException("Já existe uma instância de TileMapManager na cena.");
}
Instance = this;O desenvolvedor precisará:
- Adicionar tratamento de erro onde o TileMapManager é instanciado para capturar a nova TileMapManagerInitializationException
- Adicionar de forma semelhante o lançamento e tratamento de exceções para o caso em que os TileMaps necessários estão ausentes (não mostrado no código fornecido)
- Atualizar quaisquer testes de unidade para esperar exceções em vez de erros registrados
Original comment in English
suggestion (bug_risk): Consider throwing exceptions for initialization failures instead of just logging errors
When the manager can't be properly initialized (duplicate instance or missing required TileMaps), the current error handling allows it to continue in an invalid state. This could lead to null reference exceptions later.
Suggested implementation:
public class TileMapManagerInitializationException : Exception
{
public TileMapManagerInitializationException(string message) : base(message) { }
}
public partial class TileMapManager : Node2D
{
public static TileMapManager? Instance { get; private set; } if (Instance is not null)
{
throw new TileMapManagerInitializationException("There is already an instance of TileMapManager in the scene.");
}
Instance = this;The developer will need to:
- Add error handling where TileMapManager is instantiated to catch the new TileMapManagerInitializationException
- Similarly add exception throwing and handling for the case where required TileMaps are missing (not shown in the provided code)
- Update any unit tests to expect exceptions rather than logged errors
…o projeto e forma que o gerenciador de tilemaps pegar os tilemaps foi alterada
patrickreisx
approved these changes
Jan 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…anner para escanear o mapa atual previamente construido dentro da godot
Resumo por Sourcery
Novos Recursos:
MapScannerpara escanear o mapa.Original summary in English
Resumo por Sourcery
Novos Recursos:
Original summary in English
Summary by Sourcery
New Features: