allwpilib
allwpilib copied to clipboard
Refactor inbuilt AprilTagFieldLayout loading process
Opening PR as draft now to start discussion, but this would contain breaking changes so it should be saved for 2024.
This PR is intended to clean up the API usage when loading official AprilTagFieldLayouts. Right now, there are many ways to accomplish the same task, when there really don't need to be. This PR aims to add methods specifically for loading official field layouts for each year, allowing the total API surface (and therefore confusion) to shrink.
Open to other solutions as well, but I saw this as the simplest way to clearly present the user with the options they should be aware of (constructor for a custom one, specific methods for official layouts).
I like changing from throwing an IOException to a RuntimeException, but I'm uncertain how adding methods reduces the ways to accomplish the same task and/or shrinks the total API surface- Is the end goal to completely deprecate AprilTagFields? (Also, having the alias for the current year seems important)
but I'm uncertain how adding methods reduces the ways to accomplish the same task and/or shrinks the total API surface- Is the end goal to completely deprecate AprilTagFields?
Yea, seems like we should deprecate AprilTagFields.
having the alias for the current year seems important
Depends on whether we care about the tag layout user code expects changing out from under them when they migrate to the next year's release. Imo, changing the layout should be an explicit decision by the user.
Changing the layout definitely needs to be a conscious decision. Running your bot on a Charged Up field just to have it think it's in the wrong place because the layout went to a Crescendo field without noticing is bad. If something breaks, it needs to be noisy.
Unsure how to go about this for C++, as I am unfamiliar with the code generation that's happening there. Can the methods from AprilTagFields just be moved to AprilTagFieldLayout and aliased in some way?
Since #6326 was closed by other PRs, can this be closed?