ToME 2 maintenance

[Shany]

I understood I need this improvement when created a macro for extracting essences from first item on the floor:

Code: Select all

A:\e\e\e05ml-a

When the floor cell is empty it extracts from first item in backpack instead. It's very annoying, especially when you have there some Fireproof Book, and ordinary book of this kind is in your auto-destroy list.

[Yottle]

I could use this. I am always extracting something I shouldn't.

Now I need an auto-extract added so that when I encounter certain types of objects on the floor they automatically get extracted.

[Shany]

Now I need an auto-extract added so that when I encounter certain types of objects on the floor they automatically get extracted.

Good idea. I'll think about implementing it.

[Lord Estraven]

Hey thanks Shany, your patch fixes some borked assert() statements in the C/jansson port's squelch code. Cool... I'll import them to my fork too.

BTW, regarding the C++ port... Is there any way to make this thing compile faster? It takes about 5 times longer than the C port.

Edit: whoops, my bad... There are still assert() related crashes. Stuff like this:

Code: Select all

tome: /home/proteus/Projects/tome2/src/squeltch.c:582: condition_and_add: Assertion `c != ((void *)0)' failed.

This happens when pressing Esc while adding an automizer clause.

To be honest I don't think that assert() statement is well placed - in Angband, the Esc key means "forget I said that", so it will definitely make stuff NULL. I'm not sure yet exactly where in the code the clause becomes NULL... Ideally though, Esc should exit the clause menu, or at least do nothing at all.

I'll see if I can hack this into shape...

Edit: yeah, to get out of the menu on Esc, one can just change

assert(foo);

to

if (!foo) return;

This works fine. I'll submit a patch.

Edit 3: no, that does not work fine. There is something very wrong here; I can't nest clauses at all. As of right now it looks like the automizer is completely broken.

Also there's a second 'and' rule at the end of the rule list, which produces a segfault if selected. Not sure what's up with that yet.

Edit 4: This is just for the C port though. Not sure about C++.

[AnonymousHero]

Hey thanks Shany, your patch fixes some borked assert() statements in the C/jansson port's squelch code. Cool... I'll import them to my fork too.

(snip)
The asserts are there for a reason. It's essentially a guard to give advance warning of Undefined Behavior. UB is a very scary and damaging thing in C/C++.

Please report any particular assert failures you find as bugs.

(I'll see if I can investigate and fix the particular one you reported here.)

[Lord Estraven]

(snip)
The asserts are there for a reason.

Ah, sorry. It looked like they would fail under quite normal conditions, but yeah, I didn't check how those structs could end up being NULL.

Anyway the above assert() failure is the only one I've found in the automizer.

BTW, to answer my own question:

BTW, regarding the C++ port... Is there any way to make this thing compile faster? It takes about 5 times longer than the C port.

Turns out is is fairly easy to make ccache work with CMake. You just do this

Code: Select all

cmake -DCMAKE_CXX_COMPILER="/path/to/ccache/g++" .

Then run make, and subsequent compiles will run quite fast indeed.

[AnonymousHero]

(snip)
The asserts are there for a reason.

Ah, sorry. It looked like they would fail under quite normal conditions, but yeah, I didn't check how those structs could end up being NULL.

Anyway the above assert() failure is the only one I've found in the automizer.

Sorry, that sounded overly defensive. Of course it's very possible that the asserts could be wrong! I just meant that that's probably not commonly the right explanation . The problem is usually that the function/method is actually being called when it's not valid to call it since it's pre-condition does not hold. (Example: Asking for the first element in an empty list.)

(EDIT: Removed misleading quote fail.)

[AnonymousHero]

Btw, just as a general thing: I'm unlikely to proactively do anything about the C code at this point unless a) the problem is game-ending problems, or b) people provide a patch. I'm quite happy to apply purely bugfix patches. Functionality patches -- not so much. I don't want the 'master' branch to diverge functionally from the 'cpp' branch.

I think I'm currently at a point where the C++ branch is probably stable (and useful) enough to merge into 'master', but I'm holding off a bit longer on merging because I'm not sure when I'll have time to do a proper changelog and I have a little backlog of smaller cleanups that I'd like to do (if possible), before merging. We'll see...

[AnonymousHero]

I've just pushed a commit which should drastically reduce C++ compilation time. See the relevant commit for details.

[Lord Estraven]

Hey, thanks!

(And never mind sounding defensive; I'm not the one who spent a couple years modernizing T2.)

Re the C port - the automizer (AFAICT) works without any issues in C++, so probably better to let that stuff get fixed when merging C++ I guess.

I do have a suggestion for C++, though, which involves changes to the CMake files. My thought is, it would be good to have a target for a static Win32 binary, cross-compiled using MinGW. Currently the only way to compile for Windows is to compile on Windows, which is rather painful. With a working cross-compile target, the Windows binary could be built on Linux and tested under Wine.

[AnonymousHero]

I do have a suggestion for C++, though, which involves changes to the CMake files. My thought is, it would be good to have a target for a static Win32 binary, cross-compiled using MinGW. Currently the only way to compile for Windows is to compile on Windows, which is rather painful. With a working cross-compile target, the Windows binary could be built on Linux and tested under Wine.

If it's workable, then I'd be very happy to merge a patch that could do this! Do you happen to know what the build chain requirements are?

(I can see that Arch Linux -- which is what I use -- has some bits and bobs for cross-compiling to Win64. I have absolutely no idea how to get that working with CMake though.)

[Lord Estraven]

Unfortunately I don't know the requirements, and the distro I'm on does not have MinGW in the repos. But I'll give it a shot.

[Lord Estraven]

'nother BSD build problem in master: h-system.h includes <sys/timeb.h>. OpenBSD (as of 5.8 series snapshots) doesn't have this, so compile fails rather quickly. Without that include line, the build completes and the game runs fine.

[AnonymousHero]

'nother BSD build problem in master: h-system.h includes <sys/timeb.h>. OpenBSD (as of 5.8 series snapshots) doesn't have this, so compile fails rather quickly. Without that include line, the build completes and the game runs fine.

Any chance you could submit a MR on Gitlab? If you could test compilation on Linux too that would be ideal, but if you can't just say so, and I'll try compiling myself. (Caveat: I'm on Arch Linux, so I don't really know what non-bleeding-edge distros are doing. Yay "#include"!)

[Lord Estraven]

Hmm... Not at the moment; I don't have a dedicated OpenBSD machine, and it can't run on VirtualBox. I'll see when I can get around to it though.

Although - this was in master, with the broken automizer and all... I thought you weren't working on master any more?