Add a command to verify the title of the output ROM and throw an error if it doesn't match. for example:
expect "SUPER MARIOWORLD "
This allows us to make sure that a certain patch is only used with a certain ROM.

Apparently a function like this has existed in Asar at some point, but was removed for an unknown reason.

Macro labels can currently "leak" and affect sublabels outside of the macros. This shouldn't be the case; macro labels should be local to the macro and should disappear as soon as the macro scope is left.

Also add support for macro sublabels and macro +/- labels for consistency. Examples:

macro my_macro()

?MacroMain:
        ; ...

?.MacroSub:
        ;...

?-
        ;...
?+

endmacro

Currently running the built-in test suite requires having a clean unheadered SMW rom. This means that running the test suite in continous integration is AFAIK impossible to do legally (without sharing a ROM somewhere so the CI can access it). Currently only 11 tests need the clean SMW rom (grep '^;@\+' tests/*.asm too see which ones, mostly the ones dealing with freespace). We could replace this with a dummy ROM that is the same size as SMW but is completely garbage (it should include a few large blocks of nulls/0xFF's just to make sure the freespace finder doesn't find freespace in the original ROM space). Then we could include the dummy rom in the repo somewhere.

Requested by Ladida. Presumeably they should work just like pushpc/pullpc, except on the base instead of the pc.

ExE Boss on discord:

Also with asar, another option would be to add ' delimited strings which would be processed as is (only applying the table mapping if it’s being used).

This could quite easily be done, since most places that use quoted strings use a single dequote() function, and define expansion also happens in one place.

Make warnings more customizable like in MSVC, clang etc.

Some examples:
-Add support for "warnings as errors"
-Add support for warnings levels (print only warnigns of the current level or higher)
-Give all warnings an ID/number and add support for disabling/enable warnings per its ID
-Maybe support for disabling/enabling certain warnings or pushing/popping warnings settings from within an asm file (for example: @pushwarnings and @pullwarnings)

This could be done by adding another flag to asar_patch_ex, maybe called "should_reset" or something.
(requested by TheBiob on discord)

Building on macOS 10.13.3 gives a series of errors due to altered definition of the function strdup:

/.../asar/src/asar/libstr.h:607:15: error: exception specification in declaration
      does not match previous declaration
inline char * strdup(const char * str) throw ()
              ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/string.h:117:7: note:
      previous declaration is here
char    *strdup(const char *__s1);
         ^
/.../asar/src/asar/interface-lib.cpp:72:17: error: call to 'strdup' is ambiguous
        myerr.filename=strdup(thisfilename);
                       ^~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk/usr/include/string.h:117:7: note:
      candidate function
char    *strdup(const char *__s1);
         ^
/.../asar/src/asar/libstr.h:607:15: note: candidate function
inline char * strdup(const char * str) throw ()
              ^
... (followed by lots of other "call to 'strdup' is ambiguous" errors)
6 errors generated.

Changing the preprocessor check in src/asar/libstr.h from

#if !defined(_MSC_VER)
inline char * strdup(const char * str) throw ()

to

#if !defined(_MSC_VER) && !defined(__APPLE__)
inline char * strdup(const char * str) throw ()

solved the issue for me.

Since we have a different coding style, I would like to know what your precise coding style is, so that I can create a clang-format file and forget about this issue.

If you want, we can add this clang-format file to the repository. We could also reformat all files with it, to achieve some consistency. I tried this before, it made some of the files more readable, but this is arguable in the more macro heavy files like arch-65816.cpp.

I also think that there are some ways with CI to automatically reformat pull requests, but this is an issue for @randomdude999. I would disfavor if CI would just reject pull requests which have not been run through clang-format, since this just increases the burden on new developers.

Cppcheck report
Clang's static analyzer report
Most of these are false positives but there may be some actual bugs here. For example, I'm pretty sure the memory leak in requeststrfromuser is real (although it only leaks 256 bytes every time a string is requested, which happens like twice in total)

Namesspaces should be stackable, like in C++. Maybe add a "namespace exit" command which only leaves the current namespace rather than immediately returning to the global namespace.

Code example

namespace first

    namespace second
        Table:
            db Table ; first_second_table
    namespace exit

    Table:
        db second_Table ; first_second_Table
        db Table ; first_Table

namespace exit

Table:
    db first_second_Table ; first_second_Table
    db first_Table ; first_Table
    db Table ; Table

Add a new version of the freespace command which takes an arbitrary identifier and doesn't need an autoclean or prot to prevent freespace leaks.

Example:

org $00xxxx
    jsl NewCode

    ; ...

freespace "MyPatch"
    NewCode:
    ; ...

The returned freespace in this case would write a RATS tag, followed immediately by its identifier (in this case "MyPatch") as a 0-terminated ASCII string. Upon applying a patch twice and encountering such a freespace command, Asar could simply search the ROM for all RATS tags that are followed by that exact identifier and clean those areas.

Alternative idea: I'm not sure if having a freespace command automatically clean stuff is desirable. As an alternative, we could add a new autoclean command which takes an identifier and does exactly what I've just described. In any case this makes prot unncessary and (presumeably) makes autoclean more reliable and less complicated.

Requested by Ladida: add a feature to make Asar ignore bank crossings everywhere (instead of just certain incbin commands). Either via a command line argument or via a new command. Not sure which one would be better, but I think a new command might be, because you could disable the check only for a certain section of code or something like that.

Based on what GHB mentioned on SMWC, installing the syntax highlighting extension for Asar in Notepad++ doesn't seem to be self-explanatory, so short instructions on how to do that (as well as for the Sublime syntax highlighting) would probably be useful.

Not sure where the best place to put them would be. On one hand, the manual's focus is on Asar functionality itself, so it might seem intuitive to put that information into readme.txt. On the other hand, I think syntax highlighting is mostly irrelevant to average Asar users and is only useful to coders, so the manual might be a better place for that after all.

Rquested by GreenHammerBro in the Asar thread.

There still seem to exist a couple of undocumented features within Asar. Find them and add them to the manual.

We could use it to automatically build release files (using Appveyor) and also to verify that pull requests don't break anything (probably using Travis, but i think Appveyor might work too and IMO it would be better to use only one).

Currently, readfile()/canreadfile() functions throw an error when providing the optional parameter and trying to open a file that doesn't exist. That shouldn't be the case; the optional parameter should only be returned in those cases.

The following code gives unintuitive results.

print hex($7FFFFFFF)
print hex($FFFFFFFF)

gives

7FFFFFFF
80000000

In assembleblock.cpp we convert to unsigned int and call hex0, which in turn also expects an unsigned int. So I have no idea where this comes from.

From my observations, this should be the equivalent C code, which gives the correct result:

#include <stdio.h>
int main() {
    double f = 0xFFFFFFFF;
    printf("%X\n", (unsigned int)f);
    return 0;
};

It won't work when passing invalid options, but passing only valid options will cause a segfault. For example, asar --version. This is because libcon_option assumes that there is at least one argument remaining after all optional arguments.

Very low priority (and probably a bit messy), but it would be kinda nice to add UTF-8 support to tables, print commands and table files in a future version of Asar.

I remember working on some German patchs in the past where umlauts like ÄÖÜ caused problems and I had to write these characters as hex values instead. My VWF patch would certainly greatly benefit from this feature, but I think it would be useful to international users in general. Code compatibility should still be granted, since code uses only ASCII commands. Some strings using ANSI characters >= 128 might break from a transition, but there should only be few cases like that and they wouldn't break too horribly - only some characters would get replaced by garbage characters or disappear.

Issue pointed out by RPG Hacker in the Asar thread. Currently, math round on uses the round function, but it should use trunc to maintain behaviour similar to an int cast.

This issue doesn't occur in exe

Overview

Once an compilation fails, subsequent compilation will not succeed.

In [3], the result of asar_patch() should be true.

Opinion

Since this is an error as a result of the whole assembly, it may be doing this on purpose.

However, I think that Asar shouldn't be judge it. That's the role of a tool that uses it.

Cause

interface-lib.cpp line: 16 - errored

Initialization processing of this variable exists only in asar_math(). Why

Recently added an asar_patch_ex() function to the DLL C API, but forgot to add it to the other APIs (like C#). There are tools out there which use the C# binding in particular, so it's something that should be added before the next release.

Since you are adding new items to the asar wishlist:

How about a ASM builder API (similar to LLVM)? The idea would be that if I want to create ASM with a tool, I wouldn't need to write it to a string buffer to be read again, but instead I could write something like this:

asm_builder_label(asm, "labelname");
asm_builder_lda(asm, 0x1234);
[...]
asm_builder_bne(asm, "labelname");
asm_build(asm);

Clearly this is far fetched, will need additional thought and might as well cause a rewrite of the core of asar. I would hope that this would make the semantics of asar precise, since many of the commands in asar would map directly to those function calls.

When Asar says it doesn't think the input file is a ROM it shows the ROM title, but the shown title is only the first character of the real title repeated.

Fix

Change line 238 of interface-cli.cpp from this: unsigned char c=romdata[snestopc(0x00FFC0)];
to this: unsigned char c=romdata[snestopc(0x00FFC0+i)];

Add an include guard command like C++'s "pragma once" which makes a file only be included at most once via an incsrc.

Current solution is to use ?= for this, which works, but is ugly and impractical.

Add support to set defines via command line, like in C compilers etc. Maybe also add support to set defines via a TXT file Asar automatically searches for and includes if it exists.

Support undef command & defined() function (merge pull requests by zetaPRIME and randomdude999).

TheBiob on discord (in a DM with me):

also, telinc reminded me of another bug I forgot yesterday, If I'm correct somethings broken with the pctosnes routine for sa-1 roms
If I'm correct then my debugging a while ago showed that the 0x0F8000 can cause it to mask out the high nibble of the bank byte causing it to overwrite a completely different bank to what it's supposed to be

("the 0x0F8000" probably references libsmw.h line 157)

Not sure if this is still relevant and even if it is then how to fix it, but I think it's worth a look.

Add a filesize() function which lets us query the size of a file (to use in conjunction with readfile() functions).

see here
Also the test suite's incsrc test fails when specifying some paths (namely the folder where to find tests) relatively (all other tests work fine, i assume it's because the given path is added as a search path but the asm files to compile will be somewhere else where the relative path isn't valid anymore).
Interestingly these issues don't happen when the base file to assemble is specified absolutely.

Someone on discord mentioned that if (!a == 3) && (!b == 4) doesn't work.
This could be 'fixed' by making the comparison operators work like real operators (by adding them to the list of oper calls in math.cpp:836), and just make if evaluate its argument and check whether it's 0.
This would break compatibility with comparing strings though (which I'm pretty sure we don't want to do).
The way to fix that would be to make the entire math system use a string/double compound type (like the function parameter type), and just throw errors everywhere where numbers are expected and strings are given.

for print double(x), x can not contain parentheses (or commas, i think). The first closing parenthesis is interpreted as the end of the double() call. I'm not sure how to fix this, other than just keeping track of the parentheses yourself. Pointed out by GreenHammerBro on SMWC (although you gave him the broken solution, RPG Hacker).

• Using Cmake, the make.sh in the root of the repo is unnecessary.
• I don't think you actually need the _config.yml in docs/.
• docs/shared/highlight_js/ only needs highlight.pack.js and styles/, the other files are unnecessary.
• I feel like asar-dll-test and asar-app-test could be merged with asar-tests-shared, kinda like asar-standalone and asar are merged

Basically, readfile (line 532 @ main.cpp) gets thisfilename==<the filename we're including> so it tries to use thisfilename as the base for the path we're including which is bad. Solution (maybe): set thisfilename after calling readfile.

Would you be interested in a rewrite of arch-65816? This code would be a port of my implementation in my assembler and some kind of test bed for it, since I don't have a complete implementation of the rest of the assembler yet :).

The idea would be to replace the long if-then-else chains with a (even longer) table containing entries like:

[ OPCODE_ADC ][ PARSE_MODE_INDIRECT_Y][1] = { .byte = 0x71, .mode = ASSEMBLE_MODE_1 },

to specify that ADC (stuff),y should assemble to 71 stuff, if stuff has length 1 (the third column). ASSEMBLE_MODE_XYZ would correspond to the as_xyz macros we currently use. Then for each statement I would look up the opcode in some kind of hash map, try to match with all kinds of match("", ",y"), which gives me the parse mode, and then would make a lookup on the opcode, parse mode and length of the argument to get the correct as_xyz macro/function to call.

Sadly these tables are really long (about 400 lines), so we don't get that much of an improvement on readability, since we have a huge wall of text, which simply hides the details of the previous implementation, but it might be easier to follow what is going on, since the control flow is much smaller.

Another thing which might be difficult to implement is the opAfallback thing, which is seemingly used to be able to allow labels with the name 'A'. I wouldn't say it is impossible and would like some input if it would be okay to simply disallow labels with this name, which is definitely easier and a bit more sane if you ask me.

Currently, fullsa1rom does not support freespace searching. I think we can steal most of the code from sa1rom's freespace finder. This would make using 8 MB SA-1 ROMs (or at least the high 4 MB of the ROM) actually possible in practice.

Don't have the time to actually test it right now, but adding the issue so that I don't forget to check later.

Looking at the code of opencachedfile(), it seems as though the function isn't very smart and will most likely fail (or even crash) if we attempt to open a total of more than 16 different files in a single patch. While not likely to be done, it can technically happen and should be fixed if that's the case.

Add a resource file to the Asar DLL so that version information can be read directly from the DLL via Windows Explorer instead of only being accisble via the API.

The following assembles fine:
if 0
complete bogus
endif

In my opinion asar should raise an error here.

Add support for compiling code from a memory buffer rather than a file to the DLL interface. This let's us avoid having to create files like "tmp.asm" and similar in third-party tools. Might also slightly increase performance.

Functionality also needs to extend to file-based commands/functions etc. Proposal: add some kind of mount point that automatically makes file-based functions read from memory instead of file. For example,
incsrc "mem:/0x94FA783E"
could automatically attempt to read memory at location 0x94FA783E rather than opening a file.

Some of the stuff in our current cmake configuration is considered bad, outdated practice nowadays (for example: writing CMAKE_CXX_FLAGS directly). I've found a nice and comprehensive blog post on recommendations for modern cmake. Should be a good idea to take a look at it and improve our configuration at some point.

From the manual:

New features introduced into Asar since then might not throw warnings when attempted to use in xkas compatibility mode and old xkas patches might not assemble correctly with Asar anymore, even when xkas compatibility mode is used.

Maybe we should remove xkas support altogether? (Probably after a community discussion?)

Current file is just a leftover of old manual.txt. Have a proper Readme that doubles as a both a, proper overview for the GitHub page and a readme to bundle with future releases of Asar.

Things that should go into the Readme:

• Very basic, simple instructions for patch users how don't need to care about programming stuff ("double click Asar and it'll ask you for file names")
• Reference to the manual (ideally both the local and the online version) for patch programmers
• Shortly mention the Asar DLL and that its documentation can mostly be found in the headers of the bindings
• Reference to the changelog (ideally both the local and online version)

That's about it I think.

Is this supported? Should this be supported? How would one do this on Windows?

I tested Asar in Wine and while it could open files which I specified on the command line, it did not want to open incsrc'ed utf-8 files and wrote wrong file names when an error occured (/tmp/Σ÷ⁿ.asm:1: error: Unknown command. [├ñ├╢├╝] instead of /tmp/äöü.asm:1: error: Unknown command. [äöü])

I need this one for the normalization routine we talked about in the other thread. I can not simply do operations on single characters if the string could be utf-8.

Add support for include search paths like in C compilers etc. Should work with any and all Asar commands (and functions etc.) that reference other files (incsrc, incbin, table, readfile1() etc.).

Include search paths should be configurable via command line parameters and via a TXT file next to Asar which is automatically checked and read if it exists.

Includes should be resolved with the following priority (from highest to lowest, only applies to relative paths):

1. Check if the file exists in a path relative to the file currently being assembled.
2. Check if the file exists in any search path passed in via the command line.
3. Check if the file exists in any search path passed in via a TXT file.
4. (Maybe) check if the file exists within one (or multiple) search path(s) relative to the current Asar executable (not sure if this is a good idea or if it's better to leave that up to the TXT file)

Also make sure to support this functionality in the DLL interface (via a new function or whatever).

Basically, something like this should be possible:

function readfilenormalized(filename, pos)=readfile1(filename, pos)/255

Right now, it will throw an error because it thinks all parameters in user-defined functions are doubles. I think one simple way of fixing it could be to allow optional specification of data types in user-defined functions:

; pos defaults to ": double" here because nothing is specified.
function readfilenormalized(filename : string, pos)=readfile1(filename, pos)/255

Add a bufferfile() function which buffers an arbitrary number of bytes from a file into a memory buffer. Subsequent readfile() functions on the same file will then try to read from this memory buffer before reading any data from file. Useful when doing file I/O on larger files, might lead to performance boosts.

Some of the tests (namely v140features.asm, v150features.asm, bigincbin.asm, tablefiles.asm) fail. This is because of the way the tests are rearranged but their includes are unchanged, leading to Asar looking for them in the wrong place.