-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new features (April~May 2024) #63
Add new features (April~May 2024) #63
Conversation
This looks very impressive and well documented. I can merge this when you feel it's ready, though lemme give my 2 cents on a few things that you're (or I'm) unsure of:
This makes me very uncomfortable. I think a strong point of the EA language is that it's agnostic to the file being edited. That said,
I don't really mind this if it's functionally equivalent and the nullable annotation on references is enforced by the type system. The only thing we really lose is the ability to next the constructor (e.g.
My reading of your documentation makes me think of
Keep all of them as language defined. Same feel as
This makes me deeply uncomfortable and I am honestly not very on board with && and || making sense for how preprocessors would handle things, but not with how a language would handle things, but I think your definitions are sensible so I won't object to this.
Test cases/scripts should be included with the project, please do include them here if you have them. Also I admittedly have not been hacking or maintaining this as much in recent years, so ... don't expect the most thorough of reviews or tests for functionality or any grand visions for design. Comments I give above are mostly just my opinions / what I've seen as commonly done in project/language design. If the people using this want certain features and they're implemented, I'll merge it, the most I'll do is bugfixing here and there. |
The ability to check existing values in the binary was brought up when I asked for suggestions (by Vesly). I was not a fan of it conceptually either but I do understand the potential uses (I believe Vesly is working on randomizer mods to be applied to existing mods? I which case this could be used to check whether some known patch was installed or no). I was thinking making it require enabling the macros explicitly (probably through a command line flag) but couldn't reasonably convince myself it wouldn't just be annoying. |
…fset, nocash sym prints locals
Last three things I want to (attempt to) implement before freezing this feature-wise:
Then I will do some larger scale testing (I've been periodically testing on SkillSystem to make sure nothing breaks too horribly but I'm looking to test on other (perhaps more involved) public buildfiles) and if nothing unexpected breaks this will be ready. I guess a friendlier summary of changes could also be on the TODO list. |
…ific ASSERTs that got broken with CURRENTOFFSET changes
…reter to EADriver and EAParseConsumer to EAInterpreter
This is ready feature-wise. Any further changes would be bugfixes. Tested on Skill System and Pokemblem. Skill System builds with no changes, and Pokemblem (after addressing the diagnosed breakages) builds with some changes that relate to bugfixes which don't (seem to) break anything. I also put up a build here: https://github.com/StanHash/ColorzCore/releases/tag/20240505 |
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 strikes me that this never got merged, so I want to re-open this conversation before it bitrots any further.
It seems like @Crazycolorz5 isn't too interested in continuing to actively maintain this project,, so if it's been tested and seems to work, I don't see much of a reason not to merge it. There were some philosophical comments on whether the proposed features are a good idea (and I, for the record, agree with Colorz on most of them), but since this project has been borderline abandoned, it's doing users a bit of a disservice to hold up the entire PR over them if the project isn't otherwise moving forwards.
Sorry, I must've missed the email saying the changes were review-ready! This is good to merge, and I'll update release posts with the updated build. |
I've been hacking new features into ColorzCore for the past few weeks or so. This is getting close to ready.
I release binaries for this branch whenever here: https://github.com/StanHash/ColorzCore/releases.
Features
Some of these were suggested by other users.
Offsets are now Addresses
ORG 0x123 ; Label: ; MESSAGE "{Label:X8}"
prints08000123
, not00000123
(yes there strings now get format specifiers, see below).Symbol := value
, see below).New operators
#define IsSymbolDefined(name) "(((name) || 1) ?? 0)"
Directives and Macros
#undef
now accepts multiple parameters.#define MyData(a, b, c) ALIGN 4 ;WORD a ; SHORT b ; BYTE c
(no quotes!)#define MyName MyName
) not participate in macro expansion. This allows defining macros for use with#ifdef
and friends while still keeping the name available for symbols.#define ItemTable ItemTable
, one can doItemTable:
later without causing an infinite loop, yet also#ifdef ItemTable
is validated.Symbol assignment
{}
local scopes (unlike macros).--nocash-sym
switch is given.Formatted Interpolated strings
{expr:spec}
which are expanded within.{MyOffset:X8}
)ReadByteAt
ReadByteAt(offset)
,ReadShortAt(offset)
andReadWordAt(offset)
built-in macros. They allow reading data from the working binary. It only sees what was there before assembly.STRING and custom .TBL encodings
STRING
statement, which emits the given string encoded into a given encoding (UTF-8 by default). This makes the String(...) builtin macro redundant.STRING "my string" "<encoding name>"
"utf8"
is also correct).#inctbl
directive.#inctbl "<new encoding name>" "path/to.tbl"
Misc.
__LINE__
and__FILE__
special identifiers (closes Position macros not implemented #61)ALIGN
now takes an optional second parameter that defines an alignment offset. Basically,ALIGN Align Offset
skips bytes untilCURRENTOFFSET % Align == Offset
is true.--nocash-sym
now also outputs labels from local scopes (closes I want to create symbol information for local label. #56).BASE64
statements. This renders Base64 #59 redundant.Diagnostics
--warnings:...
.--warnings:...
allows you to disable or enable some warnings.--warnings:no-redefine
will turn off warnings when you redefine a label.--warnings:no-nonportable-pathnames
) is emitted on non-portable paths (paths with incorrect capitalization) in include/incbin/inctbl directives on Windows (closes Feature Request: Add warning when including files with nonmatching case #16).--warnings:unguarded-expression-macros
), will be emitted when a macro is expanded that features operators that aren't guarded by parenthesis.--warnings:no-unintuitive-expression-macros
) will be emitted when a macro that looks like a self-contained expression will be expanded in a way that makes it merge with its surroundings in unintuitive ways.#define MyMacro(a, b) a + b
BYTE MyMacro(1, 2) * 2
will result inBYTE 1 + 2 * 2
which would not result in what one would intuitively expect. This is particularly useful as it helps find cases where the changes in edge-case macro behavior described earlier would cause a change. An example of such case can be seen explained here: Address unintuitive macro expansion SkillSystem_FE8#637.--warnings:legacy
) is emitted when using legacy constructs kept in for compatibility (this is only emitted when using the String macro for now).Bug fixes
MySymbol : ; WORD 0 1 2 3 ... { POIN MySymbol ; ... ; MySymbol : }
the POIN would refer to the firstMySymbol
rather than the second.Technical changes
#define
,#undef
, andif[n]def
) or arbitrary tokens (#define
, maybe should be expanded to tools directives) are defined in terms of the input token stream directly.Log
is nowLogger
Maybe
(or what's left of it) is nowNullableExtensions
EAInterpreter
is nowEADriver
EAParser
class was split intoEAParser
,EAInterpreter
,StringProcessor
andLoggerExtensions
. Also the ParseAtom method was moved to an extension class calledAtomParser
.CaseInsensitiveString
was removed (it was unused)#ifdef
and#ifndef
were merged.Uncertain design points
Testing script
In the
Tests
directory lies a single python script namedrun_tests.py
. I have been working on this very basic test program in an attempt of making sure I (and possibly future contributors) don't break anything important as new stuff gets added.Things that break
ORG 1; Label:
would result in the value of Label being0x08000001
and not 1. Of course, one wouldn't need one such hack becauseLabel := 1
is now legal.ASSERT UpperBound - CURRENTOFFSET
would break if UpperBound is an offset rather than an address. This is detected and will produce a warning rather than an error if that assertion would not fail if CURRENTOFFSET was an offset.setText
now only works when one gives it a label or a raw address.Warnings have been introduced to try and diagnose most of these.