Skip to content

Implementação do ball placement simplificado#6

Open
HeitorCordeiro wants to merge 1 commit into
robocin:mainfrom
HeitorCordeiro:ball_placement
Open

Implementação do ball placement simplificado#6
HeitorCordeiro wants to merge 1 commit into
robocin:mainfrom
HeitorCordeiro:ball_placement

Conversation

@HeitorCordeiro
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@lucaasleal lucaasleal left a comment

Choose a reason for hiding this comment

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

Ótima implementação. A lógica foi bem planejada e a aplicação em código bem organizada.

Copy link
Copy Markdown

@MatheusStepple MatheusStepple left a comment

Choose a reason for hiding this comment

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

Ótimo código, apenas visualizar a possibilidade de: voltar a posição na formação original e ponto de "lateral" dinâmico.

Copy link
Copy Markdown

@giovanna-bardi giovanna-bardi left a comment

Choose a reason for hiding this comment

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

Estrutura muito boa e lógica bem estruturada. O código poderia ter um gatilho para começar o ball placement.

@joaopedrodeo
Copy link
Copy Markdown

Ótima implementação!! A lógica está correta.

Copy link
Copy Markdown
Contributor

@MatheusPaixaoG MatheusPaixaoG left a comment

Choose a reason for hiding this comment

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

Boa galera! Vi no lab e tava funcionando, deixei mais uns comentários sobre organização e padronização de código.

Comment on lines +44 to +46
QPointF targetPosition(1000, 4000);
QPointF initialPosition(2000, 0);
int tolerance = 150;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Como essas variáveis são constantes, podem ir pro arquivo CustomPlayer.h.

return;
if (!frame->has_ball())
return;
auto&& pos_ball = frame->ball();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Usar um mesmo padrão pra nomes de variáveis. Nesse caso, a variável passaria a ser posBall

QPointF initialPosition(2000, 0);
int tolerance = 150;
maxdis = 9000000;
if ((pos_ball.distTo(targetPosition)) > tolerance) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Colocar nomes descritivos para as condicionais facilita entender o que tá acontecendo:

Suggested change
if ((pos_ball.distTo(targetPosition)) > tolerance) {
bool isBallOnTargetPosition = (pos_ball.distTo(targetPosition)) <= tolerance;
if (!isBallOnTargetPosition) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Esse comentário também se aplica às outras condicionais

return;

if ((pos_ball - targetPosition).manhattanLength() > tolerance) {
if (robot->distTo(pos_ball) > 150) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Substituir o 150 por uma constante

class CustomPlayer : public Processing {
int maxdis = 9000000;
int idrobot = -1;
bool ball_placement = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Essa variável não é usada, então pode ser removida

Comment on lines +8 to +9
int maxdis = 9000000;
int idrobot = -1;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int maxdis = 9000000;
int idrobot = -1;
int maxDis = 9000000;
int idRobot = -1;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants