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

Poisoning system #63

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Poisoning system #63

wants to merge 2 commits into from

Conversation

bororeh
Copy link
Contributor

@bororeh bororeh commented May 17, 2024

Combat poisoning by MalucoBeleza.

This script was meant to maintain as much as the original scriptpack as possible.
Poisoning skill was not altered. Poison potions add the following moreZ values to blades:
Lesser poison: moreZ = 40
Poison: moreZ = 50
Greater poison: moreZ = 90
Deadly poison: moreZ = 105

Potions:
Lesser cure potions cure only lesser poison.
Cure potions cure lesser poison and poison.
Greater cure potions cure all poisons.
If you drink a potion that cannot cure your poison, it lowers the poison level by 1 (deadly poison goes to greater poison, greater poison goes to poison and poison goes to lesser poison).

Cure spells:
Cure spell cures lesser poison, poison and greater poison. If you are deadly poisoned, the poison is lowered to greater poison.
Arch cure spell cures all poisons.

Poison spells:
If you were poisoned by a weapon and receives a poison spell, the spell overrides the combat poison (same with poison field).

@Jhobean
Copy link
Collaborator

Jhobean commented Jun 12, 2024

About the events you add on the skillclass login, should we just add it on the .ini of server. for me it's look weird to add events on each login.

@Tolokio
Copy link

Tolokio commented Sep 1, 2024

poison system is already on source. This would be an override of the hardcoded poison system. As scripts is slower than source code, I think this should be at source instead, or added to scriptpack as optional, not by default.

Anyway, the script have bad optomization. Performance should be improved.

For example, it calls/search same memorys multiple time.

if (<findid.i_memPoisoned>)

            if (<argo>) && (<argo.type> == t_potion)
                if (<argo.more2> >= 1000) // greater cure potion
                    findid.i_memPoisoned.remove

                elif (<argo.more2> >= 600)  // cure potion
                    if (<src.findid.i_memPoisoned.moreZ> == 40) || (<src.findid.i_memPoisoned.moreZ> == 50)
                        findid.i_memPoisoned.remove

                    elif (<src.findid.i_memPoisoned.moreZ> == 90)
                        sysmessageua 70,1,1,1, The spell wasn't strong enough to cure your poison, but it still helped.
                        findid.i_memPoisoned.moreZ = 50
                        findid.i_memPoisoned.trigger @SetTimer

                    elif (<src.findid.i_memPoisoned.moreZ> == 105)
                        sysmessageua 70,1,1,1, The spell wasn't strong enough to cure your poison, but it still helped.
                        findid.i_memPoisoned.moreZ = 90
                        findid.i_memPoisoned.trigger @SetTimer

                    endif
                    ...

Using findid multiple times for same memory is bad. you should do something like this:

ref1=<findid.i_memPoisoned>
if <ref1.isvalid>
 ...
   //if (<src.findid.i_memPoisoned.moreZ> == 40) || (<src.findid.i_memPoisoned.moreZ> == 50)
   if  (<ref1.moreZ>==40  || <ref1.moreZ> == 50)
   ....

"Findid" look among all layers, items and containers. Avoid it as much as possible.

If is even better if u do this:

[itemdef i_memory_poison]

on=@equip
src.poison_memory=<uid>

on=@unequip
src.poison_memory=


[Events e_general_or_poison_event]
ref1=<tag0.poison_memory>
if <ref1.isvalid>
//do things here
else
 //player got no poison memory equipped
endif

With this u dont use findid at all, and tags is much better in this case than findid.

@cbnolok
Copy link
Contributor

cbnolok commented Sep 1, 2024

@Tolokio while i agree on the usefulness of optimizing that script implementation, please refrain from suggesting to place a lot of stuff in the source if you do not have an exact idea of the performance impact of a scripted implementation, which in this case is none. Source should be the leanest possible. If poisoning system wasn't already on the source for legacy reasons, i wouldn't add it again there but manage everything via scripts

@Tolokio
Copy link

Tolokio commented Sep 1, 2024

@cbnolok I haven't suggested adding anything new to the code as far as I know. I agree that the source should be as lightweight as possible. (My biggest discussions have been about this very issue.) You’ve misunderstood me (understandably). I’ll use ChatGPT to translate from now on. My English limits me.

I understand now that you are migrating to an OSI-like approach, and this is an optional (not default) script from the old system. Is that correct? If the plan is to clean up the code and put the options in the scriptpack, that sounds great to me and I begged for this in the past. I’m not informed about this. Where can I get more information?

Anyway, I’m just sharing my opinion. I don’t decide, I don’t want to decide, and I have no power over anything, nor have I ever asked for it. If you ask me not to suggest or share my opinion, you’re asking me not to participate.

I believe I have a fairly accurate idea of the impact of a script-based implementation. I’m not against its implementation, and I can agree that it’s 'practically negligible' (although everything adds up). I was just pointing out that there is already a poison system in place, and I thought this was some sort of correction to the one we already had.

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.

4 participants