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

Couple of Fixes #1222

Merged
merged 90 commits into from
Apr 28, 2024
Merged

Couple of Fixes #1222

merged 90 commits into from
Apr 28, 2024

Conversation

xwerswoodx
Copy link
Contributor

@xwerswoodx xwerswoodx commented Mar 23, 2024

SERV.LOG works same as before.
Usage of custom color system is: SERV.LOG @color,type,mask message
Example: SERV.LOG @CTCOL_RED,LOGL_EVENT,LOGM_NOCONTEXT|LOGM_KILLS message (If CTCOLs, LOGLs and LOGMs added to Defs)

cbnolok and others added 30 commits November 19, 2023 17:50
…get, but there's only one target (not attackable) in sight. (#1193)
Fixed: The issue that causes more1/2 not saved correctly for spellbook. (Issue: #1221)
Fixed: Players automatically become criminal to everyone when attack someone. (Issue: #1213)
Fixed: CallGuard not working correctly on the players that only criminal for target. (Issue: #1213)
Fixed: The wrong input of Kill trigger, to make it compatible with older sphere versions. (Issue: #1210)
Added: New Layer LAYER_STORAGE (80) to let scripters create their own storage system. (Issue: #1209)
- Only t_container and t_container_locked can be equipped to this layer.
Changed: Decay time removed from spawned item, as Sphere gives tons of invalid link errors when items decayed. (Issue: #1218)
Added: New spellflag SPELLFLAG_ASNYC to randomize field timers like old sphere versions. (Issue: #1169)
Added: New spellflag SPELLFLAG_TARG_ONLYSELF to make spell only be used on character himself. (Issue: #1171)
Changed: SERV.LOG command changed to support console colors. (Issue: #1158)
Added: Sphere.ini setting CanPetsDrinkPotion (true in default) to make pets drink potion when their owner drop it on it. (Issue: #1147)
Fixed: Missing fCheckOnly checks block the taming skill. (Issue: #1219)
Added: New trigger @drink added for characters. (Issue: #1162)
Added: New trigger @AFKmode added for characters. (Issue: #1161)
Added: New trigger @reveal added for characters. (Issue: #1157)
Added: New triggers @ArrowQuest_Add and @ArrowQuest_Close added for characters. (Issue: #1116)
Added: New function triggers f_onaccount_block and f_onaccount_unblock. (Issue: #1156)
Added: New function trigger f_onserver_broadcast. (Issue: #1145)
Added: Two new LOCAL variables ItemPoisonReductionChance and ItemPoisonReductionAmount to @hit trigger. (Issue: #1159)
Added: MAGICF_REACTIVEARMORTH (010000) MAGICF flag to Sphere.ini to let people decide if reactive armor should reflect two handed weapons.
Fixed: Possible fix for accesses and bans not loading for multis. (Issue: #1223)
@GladieUO
Copy link
Contributor

Just tried the latest version of PR and most of my system are not working. There is something different with tags returning 00 instead of 0 and same with local.x returning 01 if it should be 1. 😲 Is this intended?

@xwerswoodx
Copy link
Contributor Author

xwerswoodx commented Apr 25, 2024

Just tried the latest version of PR and most of my system are not working. There is something different with tags returning 00 instead of 0 and same with local.x returning 01 if it should be 1. 😲 Is this intended?

Yes, they should return in hex like older versions, it was a mistake that we changed it to decimal, it wasn't intentionally, so you can use dTAG to get them decimal like older versions. We discussed it on here;

#1222 (comment)

@Jhobean
Copy link
Contributor

Jhobean commented Apr 25, 2024

Yes, they should return in hex like older versions, it was a mistake that we changed it to decimal, it wasn't intentionally, so you can use dTAG to get them decimal like older versions. We discussed it on here;

I liked the new X behavior... No really agree to revert back...

@xwerswoodx
Copy link
Contributor Author

I liked the new X behavior... No really agree to revert back...

It's understandable, we can discuss about it, honestly using different system between versions, causes many script-side issues, so I agree it cause many issues when we change it in middle of development, but honestly I can't decide this by myself.

@Jhobean
Copy link
Contributor

Jhobean commented Apr 25, 2024

Since day one i'm on X I tought it was a new feature and changed all my script. Impact is big on dialog and when you see visual value or when you compare two data.
I talked about thia 4 years ago and it was say it was like this. I appreciate all work you do when bug is fixed but this change make frustrating huge difference when coding shard.

@GladieUO
Copy link
Contributor

GladieUO commented Apr 26, 2024

Im curious how it was on 56b and 56c since I dont remember we ever needed to use anything else than just simple check for tag.xxx returning decimals. Tbh taking some functionalities from the old spheres is always iffy, since we all know they were all over the place.

If in the X its working for years some way, mistake or not I think everyone using X is now used to it and cant imagine anyone appreciating this change. For servers in process, not launched, OK extra annoying work to change everything and check that it works again. But for someone already running live shard and wanting to keep with the updates, this can break a lot of stuff.

Whatever the decision will be, i guess we adapt to it and still appreciate rest of your work. 👍

Additionally I guess we can always revert this change for ourselves and recompile everytime there is new update.

src/common/CLog.cpp Outdated Show resolved Hide resolved
src/game/chars/CCharSkill.cpp Outdated Show resolved Hide resolved
src/game/items/CItemMulti.cpp Outdated Show resolved Hide resolved
src/common/resource/CResourceHolder.cpp Outdated Show resolved Hide resolved
@cbnolok
Copy link
Contributor

cbnolok commented Apr 26, 2024

Also i'd like to discuss issue #1171 since it's addressed in this pr

@cbnolok
Copy link
Contributor

cbnolok commented Apr 26, 2024

Im curious how it was on 56b and 56c since I dont remember we ever needed to use anything else than just simple check for tag.xxx returning decimals. Tbh taking some functionalities from the old spheres is always iffy, since we all know they were all over the place.

If in the X its working for years some way, mistake or not I think everyone using X is now used to it and cant imagine anyone appreciating this change. For servers in process, not launched, OK extra annoying work to change everything and check that it works again. But for someone already running live shard and wanting to keep with the updates, this can break a lot of stuff.

Whatever the decision will be, i guess we adapt to it and still appreciate rest of your work. 👍

Additionally I guess we can always revert this change for ourselves and recompile everytime there is new update.

Really does that change break so much stuff? I'm quite sure that on 56d and before everything returned hex values. That was the reason for implementing in the past dtag, dlocal etc. I don't remember when i changed that part of code, but i can say that change wasn't from the start of the fork.
I see the benefits of both returning hex and decimal, i'd say to keep the historical behavior but i'd like to understand how this revert would actually impact shards. Obviously some changes are needed to be done on scripts, but really that much?

@cbnolok
Copy link
Contributor

cbnolok commented Apr 26, 2024

Once we sort out this and the new targ spellflag, i think this pr is ready for being merged

@xwerswoodx
Copy link
Contributor Author

xwerswoodx commented Apr 26, 2024

Once we sort out this and the new targ spellflag, i think this pr is ready for being merged

I revert back the spellflag update as SPELLFLAG_FX_TARG flag enough for event, I tested it and seems like effects works with spellflag_fx flags instead of spellflag_targ flags.

For decimal/hexadecimal return, we were discussing it in discord channel.

@GladieUO
Copy link
Contributor

GladieUO commented Apr 27, 2024

Really does that change break so much stuff? I'm quite sure that on 56d and before everything returned hex values. That was the reason for implementing in the past dtag, dlocal etc. I don't remember when i changed that part of code, but i can say that change wasn't from the start of the fork. I see the benefits of both returning hex and decimal, i'd say to keep the historical behavior but i'd like to understand how this revert would actually impact shards. Obviously some changes are needed to be done on scripts, but really that much?

From the first quick look, it broken all the dialogs using any kind of tags or locals. Which in general is telling me, that I will need to go through every single tag and local condition, test ingame if its work, and if not add one letter to make it work. For me personaly that means go through everything ive been working on for year and half and marked as done and tested, test and check again.
I personaly dont see any benefits from using 16 as default, but im just lil pup to understand the purpose.

As I said, thats just me, if the change is needed or should have been done long time ago, we shall adapt.

@xwerswoodx
Copy link
Contributor Author

xwerswoodx commented Apr 27, 2024

Really does that change break so much stuff? I'm quite sure that on 56d and before everything returned hex values. That was the reason for implementing in the past dtag, dlocal etc. I don't remember when i changed that part of code, but i can say that change wasn't from the start of the fork. I see the benefits of both returning hex and decimal, i'd say to keep the historical behavior but i'd like to understand how this revert would actually impact shards. Obviously some changes are needed to be done on scripts, but really that much?

From the first quick look, it broken all the dialogs using any kind of tags or locals. Which in general is telling me, that I will need to go through every single tag and local condition, test ingame if its work, and if not add one letter to make it work. For me personaly that means go through everything ive been working on for year and half and marked as done and tested, test and check again. I personaly dont see any benefits from using 16 as default, but im just lil pup to understand the purpose.

As I said, thats just me, if the change is needed or should have been done long time ago, we shall adapt.

We decided to add h keyword (hTAG, hLOCAL, etc.) and an ini switch for default return, you can add DecimalVariables=1 in sphere.ini to make sure TAGs and LOCALs return as decimal instead of hexadecimal.

- Fixed: SPELLFLAG_NOUNPARALYZE flag wasn't working as intended.
@cbnolok cbnolok merged commit 2018b03 into Sphereserver:dev Apr 28, 2024
6 checks passed
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.

7 participants