Skip to content
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

Fix Vampyre pretending to have MP and be able to carry familiars #2

Closed
wants to merge 10,000 commits into from
Closed

Fix Vampyre pretending to have MP and be able to carry familiars #2

wants to merge 10,000 commits into from

Conversation

soolar
Copy link
Contributor

@soolar soolar commented Sep 15, 2021

I know the transition isn't actually complete yet, so feel free to wait to review this PR until it's finished. Just noticed an issue with Vampyre support that was inconveniencing me so I figured I'd fix it.

Also not sure if this is the most correct place to apply the fix. More than happy to listen to feedback on a better approach.

The branch name is no longer fully accurate because I was also annoyed by maximizer telling me to bjornify things as a Vampyre when you can't actually do that. Need to check other paths as listed in the commit message, unless others can confirm for me.

Worth noting this does not cover the case of Zombiecore, where you cannot bjornify/enthrone non-zombie familiars, since that case isn't handled by canEquip() presently (which probably warrants its own PR? I'd do it if I had any plans to run Zombiecore any time soon so I could test.)

jaadams5 and others added 30 commits April 15, 2021 16:07
git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20687 b29ace70-8910-0410-8dcc-aa2fc6433167
git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20688 b29ace70-8910-0410-8dcc-aa2fc6433167
git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20690 b29ace70-8910-0410-8dcc-aa2fc6433167
…nstead of KoLAdventure.lastLocationName because the former will still match even if you're fighting an attunement tentacle

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20691 b29ace70-8910-0410-8dcc-aa2fc6433167
…ought, its just basic delay on the zone (i.e. wanderers/tentacles do decrement it and copies of DMT monsters fought outsode of the zone do not)

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20692 b29ace70-8910-0410-8dcc-aa2fc6433167
* RestoresDatabase now uses UseItemRequest's maximumUses function for item restores
* $item dailyusesleft record now uses UseItemRequest's maximumUses function

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20696 b29ace70-8910-0410-8dcc-aa2fc6433167
git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20700 b29ace70-8910-0410-8dcc-aa2fc6433167
…ating lolmec before this patch.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20702 b29ace70-8910-0410-8dcc-aa2fc6433167
I didn't add any tests for this, but I did check that it initializes
(falling back to default behavior with no colors) if I throw an
unconditional exception beforehand, and it continues to compile and
provide me with colors if I don't have that exception in place.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20703 b29ace70-8910-0410-8dcc-aa2fc6433167
Errors are not exceptions, and _usually_ should not be caught. This
case is recoverable.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20704 b29ace70-8910-0410-8dcc-aa2fc6433167
git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20706 b29ace70-8910-0410-8dcc-aa2fc6433167
* Do not show sweet synthesis when "spleen" is filtered out of maximizer

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20707 b29ace70-8910-0410-8dcc-aa2fc6433167
…ed Hand, do not apply them for the maximizer either

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20710 b29ace70-8910-0410-8dcc-aa2fc6433167
Harvest batteries in breakfast by default

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20713 b29ace70-8910-0410-8dcc-aa2fc6433167
* Show 25 scripts at once not 15

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20715 b29ace70-8910-0410-8dcc-aa2fc6433167
Now, to upgrade our source / target versions of Java, just edit
java.version in default.properties.

Note that beyond Java 8, the leading "1." is no longer necessary,
e.g. java.version=11. (If not for this quirk, I would have edited the
target= fields instead of outright deleting them.)

Further, note that javac requires that src <= target, so we can't
build Java 15 sources targeting the Java 8 JVM.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20916 b29ace70-8910-0410-8dcc-aa2fc6433167
There are plenty more in lib/, but a number of those may be obsolete.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20917 b29ace70-8910-0410-8dcc-aa2fc6433167
This change removes the manually-included swingx sources in favor of
bundling the jar directly, in part because there are a number of
deprecation warnings in whichever version we were using.

This leads to a slight increase in the jar size (a few hundred KB by
my measurement), since we're now pulling in everything from swingx,
rather than just what we need.

There are no longer any jars that are required just for lib/ sources
(swingx is now required for both), and the two remaining jars were
actually dependencies of existing jars in src/jar.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20918 b29ace70-8910-0410-8dcc-aa2fc6433167
This reverts r20918, as frono reported that several of our frames
broke.

Revert now, fix later.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20919 b29ace70-8910-0410-8dcc-aa2fc6433167
This is effectively a revert of r20119, or attempt #2 at r20118.

Instead of using swingx-1.6, instead use 1.0, which we had been using
previously.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20920 b29ace70-8910-0410-8dcc-aa2fc6433167
Clover's mechanism for detecting test changes relies on changes to
source files, then on changed lines. Data dependencies do not trigger
either of these.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20921 b29ace70-8910-0410-8dcc-aa2fc6433167
This should be less error-prone, especially if we end up wanting to do
the same thing in other places.

gradle-clover-plugin doesn't seem to allow such fine-grained control
over test optimization, and I don't want to configure instrumentation
manually.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20922 b29ace70-8910-0410-8dcc-aa2fc6433167
As of Java 6, cross-platform tray icon support is part of the standard
library. Thus, we no longer need this special library that only worked
for 32-bit Windows anyways.

I tried this briefly on a Windows machine, and it's a little weird
(especially around clicking on it when not logged in), but it seems to
work.

Note that in order to use this, useSystemTrayIcon must be true.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20923 b29ace70-8910-0410-8dcc-aa2fc6433167
I had moved this after my initial experimentation, but didn't realize
that I needed to update where we were looking for the file.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20924 b29ace70-8910-0410-8dcc-aa2fc6433167
Ran `perl -pi -e 's/\r//g' **/*.java`, then added all the modified files.

Almost all of the rest of our code uses Unix line endings (\n), not
DOS / Windows line endings (\r\n).

I'm doing this in one big change, so the next diff is easier to read,
since one of my precommits is failing due to non-Unix line endings.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20927 b29ace70-8910-0410-8dcc-aa2fc6433167
Courtesy of fredg1.

This improves type safety, and reduces the need for casting from
Object.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20928 b29ace70-8910-0410-8dcc-aa2fc6433167
Courtesy of fredg1. This generally reduces unchecked casts from
Object, which safeguards against future misuse of these shared maps by
inserting incorrectly typed objects.

This change also reduces DebugModifers.allModifiers()'s dependence on
mutable global state, which is generally a good thing.

To test this, we use the `modifies` CLI command, which fetches all
modifiers that match some filter string. We select "water level" as
something that was used in an old path (Heavy Rains), which we expect
is unlikely to be used by new content, or conflict with future
modifier names.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20929 b29ace70-8910-0410-8dcc-aa2fc6433167
Hopefully fixed the EOL issues with my commits.





git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20930 b29ace70-8910-0410-8dcc-aa2fc6433167
…ething processed by "file not found" code. No tests yet.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20932 b29ace70-8910-0410-8dcc-aa2fc6433167
This fixes an issue with external licenses not working in the gradle
build, and also manages to reduce the jar size to be in line with
Ant's.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20933 b29ace70-8910-0410-8dcc-aa2fc6433167
Not 100% sure this is always true.
Need to check:
- G-Lover G familiars
- You, Robot with/without bird cage
- Maybe other avatars although I'm pretty sure I've already tried this with Ed in the past at least...
@soolar soolar changed the title Fix Vampyre pretending to have MP Fix Vampyre pretending to have MP and be able to carry familiars Sep 16, 2021
heeheehee-kolmafia added a commit to heeheehee-kolmafia/kolmafia that referenced this pull request Sep 16, 2021
This is effectively a revert of r20119, or attempt kolmafia#2 at r20118.

Instead of using swingx-1.6, instead use 1.0, which we had been using
previously.

git-svn-id: http://svn.code.sf.net/p/kolmafia/code@20920 b29ace70-8910-0410-8dcc-aa2fc6433167
}

// Plumber notably not included because they actually can use MP with bird calendar

Copy link
Contributor

Choose a reason for hiding this comment

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

Can Plumber use bird calendar? Conceptually I might argue against a new method that is the same as isVampyre. I'd be inclined to postpone usesMP until we had more cases it checked. Ot you could also check isPlumber and has_bird_calendar here as well. (Not real function names).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like it's better practice to have usesMP now instead of having to search through every reference to isVampyre later and figure out which ones are about the fact that it doesn't have MP and which ones aren't once we have another case. Since that is, I imagine, only a matter of time.

Also plumber might be able to use MP with something else in the future too. Heck, it might be able to use it with Source Terminal now that that path isn't standard restricted, so maybe I should update that comment to remove the bird calendar reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

From a maintainability point of view, I have some concerns/questions. If this is only used in one place then it is a waste of time and introduces a potential bug when someone is doing something with Vampyre and using text searches to assist that. Since this is basically an alias for isVampyre it will get overlooked unless/until someone realizes they need to look at usesMP as well.

I am all in favor of encapsulating something so that some future KoL change can be accommodated by one edit. But this patch isn't there yet.

I might consider a more descriptive name. Can a character in this path with these items use MP? usesMP suggests that but canUseMP might be better as long as it is understood that the current MP is not a factor (because you can't use what you don't have). If I had to pick something I might try pathCanUseMP and use a code comment to note that the availability of some items also effects the answer. Having done that I would look at the entire code base and use this instead of a path comparison when the concern is MP. If, and I haven't looked, there are other places then it would be appropriate to change those places now in the name of future proofing. If there are no other such places then it is a judgement call whether my concerns about maintainability trump your hopes of futureproofing.

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