asar
asar copied to clipboard
RPGHacker
Generating a list of features/behaviours we should deprecate
Consider this a bit of a meta issue, but the next release of asar I believe should focus on making it clear with warnings what features or behaviors could (and will) be removed in the near future. This issue is to compile such a list and tick off items as they are marked as such.
- [x] Xkas compatibility
- [x] endif for while loops (will require an endwhile be added)
- [x] quoted symbols in math parser
- [x] Warnings by number (replace with named warnings, support both during deprecation phase)
- [x] .d parsing on opcodes
- [x] xkas style math (This may not be viable, but we can add a warning still. The mega patch tests I have should let us gauge how many patches are impacted.)
- [x] overwriting builtin functions
- [x] arch_spc700_inline/arch_spc700 consolidation
- [x] Old style if !asdf
- [x] freespace
fixed - [x] Header keyword (currently unused and does nothing)
- [X] Fastrom keyword (currently unused as well)
- [X] Using \ in paths instead of / (upgrading the warning to error)
- [ ] #327 (need to deprecate
<!asdf>resolving to a named param ref) - [ ] #357
- [ ] #344
Some of these may already have warnings, but we should modify them to be more clear about pending removal. This list is up for debate (both additional items and deleting items).
DEPRECATION NOTIFICATION: Feature %s is deprecated and will be REMOVED in the future. Please update your code to conform to newer styles. Suggested work around: %s
Does this seem reasonable to everybody? If so I'll start pushing changes to checkmarked features.
Deprecation text sounds good to me. Suggested deprecation list also sounds good, aside from the "endwhile". Didn't we want to go with just a generic "end" instead, or do I remember incorrectly?
If we're going with a generic end, we'll have to teach macros to count ends and starts, instead of just looking for the endmacro.
If you guys want to do that, sounds good to me. If you don't, let's go for different ends everywhere; having both end and endmacro is just silly.
Every use of .d on an opcode is already an error, you can just delete it directly if you don't like it. Personally I'd prefer keeping it, but it makes no real difference in either direction.
Deleting xkas math is likely to break 99999 patches, but I agree, let's do it. Commands to change math parser behavior are just creepy. Kill math round too while you're at it.
~~freespace fixed sounds like it's guaranteed to not move, which can be useful if other patches target it. What replacement do you propose? Or am I guessing wrong on what it does? (I have no memories of that command whatsoever.)~~ e: replacement is freespace static. Sounds good to me.
header keyword is just xkas compat. I usually write header lorom everywhere, even though that's the default, but that's already discouraged these days because sa1. No objections from me.
generic end isn't impossible but it is likely to already be used as a label in many patches and could cause some confusion. If thats the case we need to implement end and then deprecate all other forms, plus we need to then special case several forms and create an "end stack" to track in order the things that need ended.
On the other hand I already have endwhile implementing pending commit.
Ah, yeah, good point. I forgot that programming can take effort, whoops. Yeah, let's go with endwhile then, fewer headaches that way, and probably few compatibility issues that way (since it would only effect whiles, which aren't that common).
Old style if !asdf
Actually, I vote no to this one. It would break perfectly plausible code like
!true = 1
!false = 0
!feature = !true
if !feature
Sure, you could use !true = "2+2 == 4", but is that really an improvement?
That said, I'm perfectly fine with making if 5, and anything else that's not 0 or 1, throw a warning.
e: misread again - this is unary ! on the condition, yielding things like if !!feature (if not feature). Killing that is fine, too similar to defines.
With the current commit applying all patches hosted on SMWC yields:
54 header issues 2 fastrom issues 3 endif/while 1 redefining function error
How do we want to tackle math? I'm thinking if the patch sets asar math pri (and never turns it off) no diagnostic. If math pri isn't turned on, run the calculation with both if no difference no diagnostic. If in the previous case there is a difference we can apply the original value and provide a diagnostic.
This approach may have false negatives but should catch most cases. Thoughts?
db 1/(4-2*2)
Error, or just warning?
I'm fine with both answers, as long as it's a conscious choice.
Our plan is to eventually remove math pri entirely, right? If so, I would also give the warning for when actually enabling math pri, so that people know they'll have to remove the line in a future version. Something like:
"note: math pri will be removed in a future Asar version, making standard Math priority rules always be applied"
Although then again... if we add a warning now, maybe it will lead to people removing the line prematurely and thus breaking patches. So maybe we should not warn, and then in future versions, just ignore math pri rather than outright removing it? Or rather, we should only warn on "math pri off" in the current version, and then in future versions, "math pri on" will generate a deprecation warning and "math pri off" an error?
Making both math pri on, math pri off, and db 2+2*2 warnings or errors means the easiest way to make the patch insert correctly and quietly is reverting to Asar 1.81.
I'm seconding p4s motion, with an addendum that math pri on should be a no-op in 2.0. Not even a deprecation warning; we want all warn-free 1.9 patches to be warn-free with identical results in 2.0, so we can track down whatever regressions and mistakes we make.
We can deprecate and remove it in 2.1 and 2.2. And feel free to slap on a //TODO in assembleblock.
We could make it a special "comment command" in 1.9 (since those exist) as an upgrade path. Its a bit odd, but then it allows removal without risk of breaking early (since in future versions it acts like a comment). Alternatively, math pri on could be kept one extra version and removed in a 2.1. This implies two updates to patches though, so I'll have to do an analysis as to the number of impacted patches (this could be much more significant as this is a feature that could have implications in blocks and sprites. So a single upgrade path like option one may be more preferred)
Another alternative, something along the lines of make 1.9 use math pri on by default and have a --oldmath command line flag to turn it off. Then we can have all math commands work and give warnings. This approach isn't perfect as it requires end users to be knowledgeable (not usually a good thing).
There is the possibility of adding a feature that is universally useful such that we can use it to detect a patch that is updated for 1.9 and use that as a trigger point -- a more indirect solution, but solves a lot of these problems. Maybe is the asar version command is set to 1.9 or above we use math pri on default. I think this makes the most sense as it implies an updated patch. (to be clear I am referring to ;@asar 1.9)
The last option is my preference I think, but, I left all options for the sake providing a chance for consensus (or developing one of the other plans further).
I like the ;@asar 1.9 proposal, because I think it's generally good practice to have this version specifier in a patch (kinda like how almost all CMake projects use a version specifier). It also seems like one of the least intrusive solutions and works off of a feature that already exists.
Okay so ;@asar 1.9 -> shall imply math pri on and then any use of math pri in any other context should be a deprecation warning. Sounds reasonable enough.
In the past I think we talked about arch_spc700_inline/arch_spc700 consolidation -- does anybody remember what the results of that talk was?
Upon closer inspection I think it was if (!stricmp(par, "spc700-raw")) { in the past we talked about nuking rather than inline. Though, inline is also pretty niche to NSPC engines only -- so its not inconceivable to remove it too. I can't find any patches that use inline spc support even. AMK does use raw but its a trivial upgrade path, so no problem there.
If I remember correctly, the problem with inline was that it was never even documented well, so I assume noone even understood how it worked. Maybe more people would have used it with better documentation?
Not sure if that would make for a good reason to keep it, though.
Basically, it just makes the block structure used by the NSPC engine -- it may not be a bad idea to support this through another means, but I don't think it makes sense as an arch. Isolating it as its own feature (say for example spc_block_start/end could allow us to easily support alternate engine types (either directly or through extensibility of the command itself)
I don't suppose anybody is willing to poke at warnings/errors while I work on the SPC block feature?
I would do it, but I'm at an all-time motivational low right now and don't see that changing very soon, so realistically, I think it's better for someone else to take a shot.
All features are more or less completed however custom SPC blocks are currently not supported (code partially written but I disabled them for now). gonna make a test branch I think to mess with things more.
Unfortunately I started work on this quite a few months back and got side tracked. So now its time to go back and audit the crap out of all this
What was our stance on UTF-8 again? Was that something we wanted to introduce in 1.9 (so that we can deprecate non-UTF-8 text), or was it something for 2.0+? I don't quite remember.
Anyways, I noticed yesterday that there didn't exist a changelog for 1.9 at all yet, so at some point, I suppose someone will also have to dig through old changesets and figure out everything that was changed since the last version.
My stance on UTF-8 was to hope randomdude999 figured it out. In all seriousness, I am not against seeing UTF8 in 1.9, however, I am not sure where the overall progress on this lies.
Thankfully most of the 1.9 changes are documented in this issue. Plus a few misc bug fixes.
IIRC, randomdude999 mentioned only having interest in doing the actual Asar side of UTF-8 support, while not being interested in implementing the Window API support for that. I could proably do that part, since I think it shouldn't be too much work and should be relatively straight-forward.
That being said, I don't really mind if we put UTF-8 support off until Asar 2.X. I'm not quite sure how we could implement it in a backwards-compatible way without a lot of nastiness, anyways. I think it might be easier to just implement it in a future release and then throw an error on non-UTF-8 encodings. In 99% of all cases, fixing the error would probably just require re-saving the file in a text editor, anyways.
I wonder if math rounding should be removed as well. We have floor/ceil functions for manual rounding and in general truncation happening at the last stage probably makes the most sense.