-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] Add potentialfield overview component #5
base: master
Are you sure you want to change the base?
Conversation
| .findFirst() | ||
| .orElse(new RGB(0, 0, 0)); | ||
| graphics.setBackgroundColor(background); | ||
| graphics.setForegroundColor(this.visibleTextColor(background)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not put these two calls in a map after findFirst()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it doesn't matter. The map is only evaluated once as far as I know. I will test this an come back to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified;
IntStream.range(0, 10)
.map(i -> { System.out.println(i); return i;})
.findFirst();Simply prints 0 as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm sorry, i meant ifPresent:
component.getBackgrounds().stream()
.map(b -> b.apply(position))
.filter(Objects::nonNull)
.findFirst()
.ifPresent(background -> {
graphics.setBackgroundColor(background);
graphics.setForegroundColor(this.visibleTextColor(background));
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise i don't have a default background/foreground color
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you need it, shouldn't it be configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be; but the logic remains the same. Since one point may return values, another may not. If another point doesn't return a specific value it should use the default. But if you only set the default at the beginning as your example suggests; you'll re-use the colors as they were in the previous pass.
| */ | ||
| public class PotentialBackground implements Function<TerminalPosition, TextColor> { | ||
| private final Function<Double, TextColor.RGB> gradient; | ||
| private final INDArray data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the sampling mechanism (FieldGenerator) should be part of this class or be generalized and move to algieba
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that this data field is acquired at the end of our potentialfield-strategy. I don't know how we can effectively couple it to be honest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure what you mean by that, because the value is calculated in single points if it's needed for the strategy. if you need a sampling of the entire field then that is unrelated to the strategy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but essentially we need to calculate a a subset of all values for the gui; so the gui should receive or be able to retrieve the necessary dataset at one point.
| ){ | ||
| this(character, players.stream() | ||
| .filter(r -> CardinalDirection.from(r.getOrientation()).equals(cardinalDirection)) | ||
| .map(robot -> offset.withRelative((int)robot.getX(), (int)robot.getY())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not Math.floor? if you need rounding then typecasting will not work the same for negative numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Math.floor returns the same type; i need to convert the double to an int. for now i don't care too much about whether it's floored or rounded unless there's a good reason to care :)
| * @author Jeroen de Jong | ||
| */ | ||
| public class PotentialBackground implements Function<TerminalPosition, TextColor> { | ||
| private final Function<Double, TextColor.RGB> gradient; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: change the second generic to an interface instead of a class. Torch should be updated to make Gradient return TextColor first though.
| ), | ||
| Arrays.asList( | ||
| new OrientationIndicator(robots), | ||
| balls.stream().collect(Collectors.toMap(b -> new TerminalPosition((int) b.getX(), (int) b.getY()), b -> 'B'))::get, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want this to be less ambiguous though. @romnous Ideas?
| Arrays.asList( | ||
| new OrientationIndicator(robots), | ||
| balls.stream().collect(Collectors.toMap(b -> new TerminalPosition((int) b.getX(), (int) b.getY()), b -> 'B'))::get, | ||
| robots.stream().collect(Collectors.toMap(r -> new TerminalPosition((int)r.getX(), (int)r.getY()), r -> String.valueOf(r.getId()).charAt(0)))::get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also need a better way to turn an int (1-13) into a (hexadecimal) char (1-C)...
| graphics.setBackgroundColor( | ||
| player.getTeamColor() == TeamColor.BLUE ? ANSI.BLUE : ANSI.YELLOW); | ||
| graphics.setCharacter((int) player.getX(), (int) player.getY(), | ||
| String.valueOf(player.getId()).charAt(0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
graphics.putString((int) player.getX(), (int) player.getY(), Integer.toHexString(player.getId())should be fine
| * @author Jeroen de Jong | ||
| */ | ||
| @AllArgsConstructor | ||
| public class Robots extends AbstractComponent<Robots> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be RobotPositions or maybe even RobotPositionsComponent :')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd prefer suffixing with Component
| @Override | ||
| public TerminalSize getPreferredSize(final OrientationIndicators component) { | ||
| // TODO This should be determined based on the set of players | ||
| return new TerminalSize(10, 20); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe component.getParent().getPrefferedSize() could suffice.
| }))); | ||
| } | ||
|
|
||
| private Character getCharacter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RyanMeulenkamp Could you figure out a more beautiful solution for this? It's code to put either an -, | or + for a line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least you could turn that ternary into else if and else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was an ternary expression; but that wasn't much clearer as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no i mean
if ((fieldLine.getYStart() == y && fieldLine.getXStart() == x) ||
(fieldLine.getYEnd() == y && fieldLine.getXEnd() == x)) {
return '+';
}
else if (fieldLine.getYStart() == fieldLine.getYEnd()) {
return '|';
else {
return '–';
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally failed in parsing that sentence.
| }))); | ||
| } | ||
|
|
||
| private Character getCharacter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least you could turn that ternary into else if and else
| final S data | ||
| ) { | ||
| final BiFunction<Integer, Integer, TextColor> backgroundSupplier = new GaussianPotentialFieldBackground( | ||
| (GaussianPotentialField) data.getPotentialField(), new Gradient(Color.GREEN, Color.RED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PotentialField.Supplier in algieba should take a generic type to fix this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to release it before i can remove the cast though.
|
|
||
| @Override | ||
| public TextColor apply(final Integer x, final Integer y) { | ||
| final INDArray positionVector = potentialField.getPotential(Nd4j.create(new double[]{x, y})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be called potential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't forget state vectors are column vectors. if you don't specify the dimensions of the array then nd4j will give you a row vector. try this:
potentialField.getPotential(Nd4j.create(new double[]{x, y}, new int[]{2, 1}));
| * @author Jeroen de Jong | ||
| */ | ||
| @AllArgsConstructor | ||
| public class Robots extends AbstractComponent<Robots> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd prefer suffixing with Component
Todo
Might want to find a way to make font boldList<Function<x, y>>-findfirst logic.List<Function<X, Y>>classes. Such asPotentialBackgroundandTeamColorBackground.TeamColorBackgroundsort the players on teamcolor.I might wantPotentialBackgroundto returnnullif the value in theINDArray== 0. That allows for a different default background (field-green or something)