-
Notifications
You must be signed in to change notification settings - Fork 85
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 some Gothic 1 magic bugs #682
Conversation
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.
Hi, @thokkat and thanks for PR!
(18.1/2.) Spell targets can be restricted ...
This part seem to be a reference?
game/world/worldobjects.cpp
Outdated
@@ -995,6 +999,27 @@ static bool checkFlag(Interactive& i,WorldObjects::SearchFlg f){ | |||
return true; | |||
} | |||
|
|||
template<class T> | |||
bool checkTargetType(T&, TargetType) { return true; } |
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.
Function can be static
game/world/objects/item.cpp
Outdated
if(!isSpellOrRune()) | ||
return false; | ||
bool g1 = world.version().game==1; | ||
if(g1 && spellId()==47) |
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.
nitpick: better to decouple 47
in something like const int32_t SPL_STORMFIST = 47
bool checkTargetType(T&, TargetType) { return true; } | ||
|
||
static bool checkTargetType(Npc& n, TargetType t) { | ||
if(bool(t&(TARGET_TYPE_ALL|TARGET_TYPE_NPCS))) |
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.
Is this code also G1 only?
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.
No can be used for G2. SPL_WindFist
has TARGET_TYPE_NPCS
. If changed to TARGET_TYPE_UNDEAD
novices can't be targeted anymore.
Here it's just a leftover because non-npcs can't be targeted anyway. Mostly if G2 does something different G1 stuff is still functional but unused,
Meant issues 18.1 and 18.2 in bug discussion
|
Merged, thanks! |
Not sure if it is correct to comment after succesful merge.
what I understood you have fixed here is that destroy undead cannot focus on enemies. While it appears to fix the problem, the player can still kill Grash-Varrag-Arushat as well as MC warriors etc. with that spell by aiming free (without focusing). Standing directly infront of G-V-A and charging the spell still kills him even if not focused. |
BIG thank you for this PR @thokkat! |
This PR fixes issues 1. and 2. from #661 and 18.1 and 18.2 from G1 bug discussion.
(1) Broken firerain effect is fixed by adding it's
Target_collect_algo
attribute toItem::isSpellShoot
. I proposed this change some time ago but withdrew because it would break Stormfist. Found out now that projectile property is hardcoded (source) and used that for Stormfist.(2) Added a missing perc trigger on collision to make icewave freezing work.
(18.1/2.) Spell targets can be restricted based on attribute
Target_collect_type
, meaning orc priest just cannot be targeted. ForTARGET_TYPE_UNDEAD
I tookC_NpcIsUndead
script function from G2 as reference since it doesn't exist in G1.Target type is not always correctly set in vanilla. For example icewave can target items.
While at it also limited focus search for interactives in combat to only bow and crossbow as magic can't target those.