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

Initial Release of Equipment Switch #3548

Merged
merged 23 commits into from Dec 24, 2018
Merged

Initial Release of Equipment Switch #3548

merged 23 commits into from Dec 24, 2018

Conversation

Lemongrass3110
Copy link
Member

@Lemongrass3110 Lemongrass3110 commented Oct 4, 2018

  • Addressed Issue(s): cant find the correct one - if someone does please link it.
  • Server Mode: renewal
  • Description of Pull Request:
    First implementation of the new kRO feature.

Thanks to everyone who contributed to this release in any way, be it donations, information or testing.

Thanks to everyone who contributed to this release in any way, be it donations, information or testing.
Since our PACKETVER is not supporting it this is required for CI
@anacondaq
Copy link

@anacondaq anacondaq commented Oct 4, 2018

Okay, founded problem (I think people will report about it with a time)

It's because of MAX_STORAGE > 850, while almost all uses 900-1000 slots for storage. I think these people will report about the issue. I don't think it's possible somewhat to do with it instead of decreasing max storage size a little bit.

@Lemongrass3110
Copy link
Member Author

@Lemongrass3110 Lemongrass3110 commented Oct 4, 2018

@Anacondaqq yeah totally forgot about this, since it has been this long already. I think the new maximum was somewhere between 820~850 items.

@@ -67,6 +67,10 @@ feature.roulette: on
// Requires: 2015-05-13aRagexe or later
feature.achievement: on

// Equipment Switch (Note 1)
// Requires: 2017-02-08bRagexeRE or later
feature.equipswitch: off
Copy link
Contributor

@Atemo Atemo Oct 4, 2018

Choose a reason for hiding this comment

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

The feature is not available on official server by default?

Copy link
Contributor

@aleos89 aleos89 Oct 5, 2018

Choose a reason for hiding this comment

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

I think this is because of the warning that’s presented at start up since our packetver is 2015~ and it’s causing the build bots to “fail”.

@Balferian
Copy link
Contributor

@Balferian Balferian commented Oct 5, 2018

Solved - Thanks to @aleos89
When server starts I have some SQL errors with cart_inventory.
All work fine, but need to update upgrade_20181004.sql
add column to cart_inventory:favorite and equipSwitch.
Do we need column "favorite" in cart_inventory? Oo

@aleos89
Copy link
Contributor

@aleos89 aleos89 commented Oct 5, 2018

Sounds like you applied the diff wrong. Cart Inventory doesn't have a favorite tab so that column is not needed in SQL. Look at the diff again and make sure it only applies to TABLE_INVENTORY when parsing char-server side.

@MathReaper
Copy link
Contributor

@MathReaper MathReaper commented Oct 7, 2018

My client stop "updating the receive packet" after trying to equip something.

I've received the packet: 9A 0A 02 00 02 00 00 00 00 00 98 0A 0F 00 22 00 00 00 00 00 (size 20)

0x0A9A and 0x0A98 aren't different packets? The client understand that are two packets in only one recv buffer? Interesting.

I can't understand why my client "stops" when I receive the packet 0x0a98, lol.
The only thing that happens is that I receive an error message saying: Unable to register item. Wth?

Client date: 20170215

#if PACKETVER >= 20170208
parseable_packet(0x0A97,8,clif_parse_equipswitch_add,2,4);
packet(0x0A98,10);
parseable_packet(0x0A99,4,clif_parse_equipswitch_remove,2,4);
Copy link
Contributor

@MathReaper MathReaper Oct 7, 2018

Choose a reason for hiding this comment

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

Trying to unequip boots:
[Warning] clif_parse: Received unsupported packet (packet 0x0040, 4 bytes received), disconnecting session #3.

0x0A99 have size 8 on client 20170215.

Copy link
Member Author

@Lemongrass3110 Lemongrass3110 Oct 10, 2018

Choose a reason for hiding this comment

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

Sorry I do not have that client to confirm this. :(

Copy link
Contributor

@MathReaper MathReaper Oct 11, 2018

Choose a reason for hiding this comment

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

I can upload here. I will open a repo of clients unpacked by myself soon.
2017-02-15aRagexeRE - MediaFire

Copy link
Member Author

@Lemongrass3110 Lemongrass3110 Oct 16, 2018

Choose a reason for hiding this comment

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

This has been fixed as well. :)

@BruSantato
Copy link

@BruSantato BruSantato commented Oct 7, 2018

I added the correct Diff all in a clean rAthena, when connecting the emulator does not have any error, but it is not changing the equipments when clicking on CHANGE
I use a client: 20180530

image

@anacondaq
Copy link

@anacondaq anacondaq commented Oct 7, 2018

@BruSantato check feature.conf feature.equip_switch must be on, by default it's off.

@BruSantato
Copy link

@BruSantato BruSantato commented Oct 7, 2018

@Anacondaqq Sorry, I forgot to mention this, it's already activated, and even then it does not make the exchange of equipment

@MathReaper
Copy link
Contributor

@MathReaper MathReaper commented Oct 8, 2018

2H weapons actually conflicts with 1H weapons or shields (switching 2H weapon for weapon + shield) and the 1H weapon is removed from equipment switch interface.

@MathReaper
Copy link
Contributor

@MathReaper MathReaper commented Oct 11, 2018

@BruSantato I'm just reporting an issue, this isn't related to what you're reporting. xD

ghost referenced this issue Oct 13, 2018
@ghost
Copy link

@ghost ghost commented Oct 14, 2018

Not sure if this is intended but if you try to change equip from 2Handed Equipement with Normal Weapon + Shield it fails. (In-game no error thought and in console)

HTR: Equip 2H Sword -> Set shield and knife to replacement window -> Use the feature still the same.

See image attached.
1
2
3
4
5

@rathena rathena deleted a comment from Ryuhorox Oct 14, 2018
@MathReaper
Copy link
Contributor

@MathReaper MathReaper commented Oct 14, 2018

@ecdarreola I think that this happens because pc_equipswitch function is called two times by the skill because the equipment is in another position too, and the weapon is switched 2 times?

@lukasrmattos
Copy link

@lukasrmattos lukasrmattos commented Oct 30, 2018

Test:

I have the problem that after changing one time the last items are not more change able without drag this again. Is this the default behavior?

https://vimeo.com/297988385

@BrOgBr
Copy link
Contributor

@BrOgBr BrOgBr commented Oct 30, 2018

@zellkennedy I think that this problem was already reported, it's because the two-handed weapons. ;)

@lukasrmattos
Copy link

@lukasrmattos lukasrmattos commented Oct 30, 2018

@zellkennedy I think that this problem was already reported, it's because the two-handed weapons. ;)

Hmm, @Lemongrass3110 sayed that was fixed but still, I can reproduce it and ecdarreola problem is other.

@mjonrest
Copy link

@mjonrest mjonrest commented Nov 3, 2018

error
i found this error.. this is cause by monster that spamming waterball like drake,high wizard katrinn,strouf,etc

@BruSantato
Copy link

@BruSantato BruSantato commented Nov 18, 2018

Hello, I just applied the system in a clean rathena, the items are added to the replace slots but I can not change the items, the system is active in features.conf and the sql table is imported
I'm using a client 2018-06-20 and the system does not work
teste replace equipament

@RadianFord
Copy link
Contributor

@RadianFord RadianFord commented Dec 3, 2018

I've tested this one and the only problem was.

1. Place main gauche in the replacement equipment.
2. Equip infiltrator in the normal equipment window.
3. Switch/Change equipment's.
4. Result: dagger switched perfectly.
5. Switch it again into the infiltrator weapon.
6. Result: Dagger will go back to normal state.

@Mateuus
Copy link

@Mateuus Mateuus commented Dec 7, 2018

For those who are in trouble not working with 2018-06-20
Comment this part

clif.cpp:

/*
	if( DIFF_TICK(tick, sd->equipswitch_tick) < 0 ) {
	// Client will not let you send a request
		return;
	}
 	sd->equipswitch_tick = tick + skill_get_cooldown( skill_id, skill_lv );
 	*/

@RadianFord
Copy link
Contributor

@RadianFord RadianFord commented Dec 18, 2018

I noticed somethings.

  1. If mapflag noskill are set, of course this wont work since its a skill.
    Suggestion: add map_msg The Switch Feature wont work in this map.

  2. If your character is sitting, this wont work either.
    Suggestion: add map_msg The Switch Feature wont work while in sitting mode.

# Conflicts:
#	src/common/mmo.hpp
#	src/map/battle.cpp
#	src/map/battle.hpp
#	src/map/clif.cpp
#	src/map/clif.hpp
#	src/map/pc.hpp
@rAthenaAPI
Copy link
Contributor

@rAthenaAPI rAthenaAPI commented Dec 20, 2018

This pull request introduces 3 alerts when merging 8999c56 into 01f61cf - view on LGTM.com

new alerts:

  • 3 for Comparison result is always the same

Comment posted by LGTM.com

This will remove duplicated code.
Tested on officials. This skill works around this mapflag.

Thanks to @RadianFord
@Lemongrass3110
Copy link
Member Author

@Lemongrass3110 Lemongrass3110 commented Dec 21, 2018

@RadianFord it is actually easier to reproduce.
Dagger on main window -> Katar on switch window -> switch

@Lemongrass3110
Copy link
Member Author

@Lemongrass3110 Lemongrass3110 commented Dec 21, 2018

So guys everything should be fixed now.
Please test it again and if no further reports come in, we can merge this to master after review.

@RadianFord
Copy link
Contributor

@RadianFord RadianFord commented Dec 21, 2018

@Lemongrass3110 Yeah, i just found out that tooday..

If a monster or anything else cast the equip switch skill it could crash the server.

Thanks to @mjonrest
Removed TODOs
@Lemongrass3110 Lemongrass3110 merged commit 818ff10 into master Dec 24, 2018
6 checks passed
SeravySensei pushed a commit to SeravySensei/rathena that referenced this issue Jan 26, 2019
Thanks to everyone who contributed to this release in any way, be it donations, information or testing.

Merry Christmas :-)
@Lemongrass3110 Lemongrass3110 deleted the feature/equip_switch branch Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet