Skip to content

2.0 Beta 1 Part 3 #722

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

Open
wants to merge 203 commits into
base: master
Choose a base branch
from
Open

2.0 Beta 1 Part 3 #722

wants to merge 203 commits into from

Conversation

NQNStudios
Copy link
Collaborator

@NQNStudios NQNStudios commented Apr 10, 2025

This was all innocent enough minor fixes until the 3 commits before the last one. Then things got weird.

As I'll be pushing more later let me specify the commits that were major weird stuff...

Remember window positions as preferences
Let the minimap be hidden by other applications in focus
WIP standardize mouse position translation -- The idea of this one was to fix #721 while also making future misalignments between visual selection and actual selection less likely to happen. I have not tested enough to be confident that it did fix the bug.

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented Apr 11, 2025

I'm deep in territory I don't wanna be in. Very yikes.

EDIT: What I mean by that is that I've been making fundamental changes to the input system, which recording and replay can't possibly test. All of these changes cascaded logically out of wanting to fix #721 in a comprehensive, future-proof way. But the changes took a lot of mental wrestling to figure out and I slipped into "Move fast and break things" territory which means I probably made mistakes that could break all kinds of things. I need to go back over these changes very carefully, and if I decide to keep them, it invalidates a lot of the testing I've done, setting me back from approaching a well-tested release candidate.

Comment on lines 338 to 349
if(item.missile <= 0)
show_charges &= item.ident;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the missile graphic number? Is that really a good metric…?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a great one. The other option I considered was checking variety for whether the item has one of several missile varieties. I went with this because it makes for a simpler check. I want to avoid adding at xml tag that specifies that it should show a quantity when unidentified.

@NQNStudios NQNStudios force-pushed the 2.0b1-c branch 5 times, most recently from 9c59955 to 4d09773 Compare April 23, 2025 01:16
@CelticMinstrel
Copy link
Member

Are you certain through_sending() wasn't doing anything useful? My current impression of it is that it was intended to slow down the progress of the text buffer if a huge amount of text is added at once, to give you a chance to see it. I don't know if it was actually doing that, though.

@NQNStudios
Copy link
Collaborator Author

I removed the functions I removed because I grepped and saw nowhere that would have triggered them being called, either by calling them directly or by setting bools that would result in it happening. That said, I didn't thoroughly look into what those functions were supposed to do, and so it bears looking closer to be more than 90% sure I didn't remove anything that would be good to keep.

@CelticMinstrel
Copy link
Member

I think most special nodes would be previewable, so maybe the preview flag should default to true and be explicitly disabled via .no_preview() on nodes that can't be previewed.

@NQNStudios
Copy link
Collaborator Author

I think most special nodes would be previewable, so maybe the preview flag should default to true and be explicitly disabled via .no_preview() on nodes that can't be previewed.

Well, right now each type of node needs to have its preview manually implemented, so inverting that flag would just make the button appear for a bunch of node types that aren't actually previewable until that work gets done. I might be splitting this work into a separate PR and not implementing every type of preview right away.

@CelticMinstrel
Copy link
Member

Well, right now each type of node needs to have its preview manually implemented, so inverting that flag would just make the button appear for a bunch of node types that aren't actually previewable until that work gets done.

There is a sensible default for a node that doesn't specify a preview method, though – use msg1 and msg2 for a 2-string dialog.

@NQNStudios
Copy link
Collaborator Author

Good idea, I'll put that in and see how many cases it adequately covers.

@CelticMinstrel
Copy link
Member

2f2f61b probably should not have changed the filesize, yet the size is more than double. Can you explain that? The files should be saved without needless metadata and in indexed 8-bit format.

@NQNStudios
Copy link
Collaborator Author

2f2f61b probably should not have changed the filesize, yet the size is more than double. Can you explain that? The files should be saved without needless metadata and in indexed 8-bit format.

That will be for the same reason it happened last time--default GIMP PNG export settings. How did you end up exporting it to get the size down again last time?

I'll fix it soon but another change is coming to the same file, I have someone working on a more visible baby hydra.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented May 1, 2025

How did you end up exporting it to get the size down again last time?

I think I unticked every box in the export settings dialog, but I don't remember if I did anything else in addition to that.

I'll fix it soon but another change is coming to the same file

If you're rebasing and squashing anyway it's probably better to erase the commit where it inflated.

@NQNStudios NQNStudios force-pushed the 2.0b1-c branch 4 times, most recently from cad5b07 to 3354a41 Compare May 2, 2025 21:26
@NQNStudios
Copy link
Collaborator Author

efab3ee I fixed the size inflation of monst10.png and added a darker baby hydra I had a friend make. We agree it could be prettier but so could all the other sprites--this at least fixes the visibility.

@NQNStudios NQNStudios force-pushed the 2.0b1-c branch 4 times, most recently from a3b7bff to a459e9b Compare May 6, 2025 03:06
@CelticMinstrel
Copy link
Member

CelticMinstrel commented May 8, 2025

I don't like this: 1aeac0c

If you want an acid damage type, then add an acid damage type. Don't just half add it. And why on earth is it -1? That makes no sense. Also, if you're adding it then it needs its own boom. Maybe as a quick temp graphic you can just invert the magic boom.

Personally, I don't think we really need a separate acid damage type. I don't especially mind having one either, though.

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented May 8, 2025

And why on earth is it -1? That makes no sense.

The next ones say 'keep these two last'--I am afraid of changing their values (or any of the values in this enum) because magic number references to them could be anywhere.

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented May 8, 2025

I'm still processing it as magic damage so as not to change the game balance--magic resistance has always applied to it, and there are no features anywhere that would add acid resistance. The reason for "half-adding" it now, is mainly to use the intended sound effect and make it easy to add a graphic for it later.

EDIT: Oh, yeah, I can see what I could come up with as a quick temp graphic, yes.

@CelticMinstrel
Copy link
Member

Well, this commit changed SPECIAL from 9 to 8: d877df5

So that suggests to me that changing it back to 9 shouldn't cause any problems.

@CelticMinstrel
Copy link
Member

I'm still processing it as magic damage so as not to change the game balance--magic resistance has always applied to it, and there are no features anywhere that would add acid resistance.

Its existence in the enum means you can set acid resistance and whatever other things you can come up with.

I think the correct approach is to handle it like I handled the new races (Vahnatai, Goblin, Skeletal Undead, possibly a few others) – basically, anything that checks for magical also covers acid. So, magic resistance also gives acid resistance, and protection from magic also protects from acid, etc.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented May 8, 2025

Re: 1d3a641

For this you probably need to update unit tests and maybe some of the sample scenario files in the test folder.

Oh, and the scenario schema file as well. Would be nice if you can make a new schema for the editor file too…

NQNStudios added 2 commits May 8, 2025 07:57
this is adapted from a BoA monster graphic, I think created by Andrew Hunter,
who is already in the credits as the primary artist
@NQNStudios
Copy link
Collaborator Author

For a temporary acid boom, I started with this monster graphic from BoA:

image

I isolated the slime, then scaled it down to fit BoE boom size.

BoA credits two artists, one for the line art, and one for the icons. Andrew Hunter is the icon artist, and already credited as the main artist for BoE. I think somewhere you said we already have Andrew Hunter's permission to use additional graphics he made for Spiderweb games? Anyway, we can always ask to make sure.

NQNStudios added 3 commits May 8, 2025 17:28
multiple places in the code called boom_space for outdoors, but boom_space
had early returns in outdoor modes.
@CelticMinstrel
Copy link
Member

For a temporary acid boom, I started with this monster graphic from BoA:

Uhh… what? I think using graphics from BoA is fairly questionable – I even left out a bunch of graphics that Mistb0rn created specifically for BoE because of that. Besides that, the graphic doesn't even match the aesthetics of the other booms. That makes it a really bad fit even as a temp graphic.

Maaaybe you could get something useful out of that slime if you mirrored it vertically and squished it in just the right way (not uniformly – the protrusions would need to shrink more than the centre), but I'd much rather take one of the pre-existing booms on that sheet and modify it.

BoA credits two artists, one for the line art, and one for the icons. Andrew Hunter is the icon artist, and already credited as the main artist for BoE. I think somewhere you said we already have Andrew Hunter's permission to use additional graphics he made for Spiderweb games? Anyway, we can always ask to make sure.

We have Andrew Hunter's permission to use graphics he created for the Exile trilogy. I don't know if that permission extends to graphics created for the Avernum series. I didn't even know he had created those graphics.

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented May 8, 2025

I reeeeaaaally don't think it sticks out so horribly as you're saying. Just a reminder that I'm doing my best and trying to make as many improvements as I can over here with limited time to work. I tried fiddling with the pre-existing booms and couldn't come up with anything that felt distinct enough. I do think what I came up with is decent.

EDIT: Not saying no to the criticism, I just feel some of these reactions lately imply what I did is utterly baffling--in an iterative process where things aren't finished, no less.

@NQNStudios
Copy link
Collaborator Author

Also, damaging terrains write out their damage type in terrain.xml as an integer, so making changes to eDamageType values could definitely be destructive.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented May 9, 2025

Also, damaging terrains write out their damage type in terrain.xml as an integer, so making changes to eDamageType values could definitely be destructive.

That should be fixed, but there's no need to worry about it here – there won't be any damaging terrains that do special damage in any scenario.

Resist Magic used to not help with magic damage!

I think the point of resist magic was improved saving throws, not reduced damage. (Assuming you mean the status effect.) That said, I don't have any problem with changing that, especially considering it does reduce fire and ice damage.

add_string_to_buf(" It's acid!"); // Maybe?

How about "It burns!"? Or something along those lines.

I reeeeaaaally don't think it sticks out so horribly as you're saying. Just a reminder that I'm doing my best and trying to make as many improvements as I can over here with limited time to work. I tried fiddling with the pre-existing booms and couldn't come up with anything that felt distinct enough. I do think what I came up with is decent.

I don't really know if it could be called decent, but I do know that it doesn't really fit in with the others and I don't really like it.

I had a thought that the ideal boom would be a larger version of the acid status icon (since the poison boom and status icon already match). I tried fiddling a bit on making the poison boom be more like the acid status icon, but couldn't come up with anything good.

I just feel some of these reactions lately imply what I did is utterly baffling--in an iterative process where things aren't finished, no less.

I truly was baffled when you picked something from Blades of Avernum… I can't think of anything else that counted as truly baffling though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rotated wall spells don't always put the wall where the targeting reticle shows
2 participants