In that case I propose some kind of automatism, since manual handling of all these deprecation notes and dialogs will be way too much work. Something like a special format for the deprecation comment and the date in it, and a code scanner that regularly scans the codebase, finds these comments and reports which of them are due for deletion.
Maybe even a custom annotation, something like
@Deprecated
@VassalDeprecated(since = "2020-08-05")
void method() {
}
This actually looks cool, I’ve never written custom annotations but they are powerful, I think we could hide the whole code for showing the dialog behind them. Then a custom maven plugin that compares the date in these annotations with current date and spits out a warning if a year has passed.
One question that remains is, how to handle deprecated fields? In a method call we can simply add the code for showing the dialog but how do we do that for access of deprecated fields? Could custom annotations handle that? I don’t know.
Another question that I have right away if we do this, can I go on and mark 99% of all protected fields and methods as deprecated? Just in case we might want to delete them in a year. Many (or even most?) of the protected fields and methods are not even meant to be called by subclasses, many classes that have these fields are not even meant to be subclassed, they were just made protected due to … I don’t know, old-school object-oriented approach where everything is per default protected? We have this spotbugs plugin now for instance and it detected a lot of “protected static” fields that should be final but are not, they are constants, we could and should make them final but we don’t know for certain whether modules assign new values to them. If they were private it would be easy going but thanks to this “protected by default” way of writing code we have a lot of trouble managing this code, and a lot of ways for custom modules to break Vassal by simply assigning new values to core constants.
Small example to make things more clear:
So in a module I write this class and break core functionality, thanks to glorious protected visibility:
class VassalBreaker extends ImageIOImageLoader {
public VassalBreaker() {
readImage = null;
readSize = null;
}
}
Now some will answer “but that’s highly hypothetical, no module should and would do such a thing” but I ask, why then allow this in the first place, why the F make these things protected, why open doors instead of hiding core code behind safe walls?
Overall, I am ambivalent on this issue. Generally I am for marking much more as deprecated and deleting it ASAP, or hiding it from the public API by changing protected to private. But if we do this one-year thing I am for investing some more time and thought up front and building a decent “deprecation system”, and then having a smooth ride in the years to come where we just get notified by the build server of what can be deleted.