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

stopselling not cleaning items #2909

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dastgirp
Copy link
Member

@dastgirp dastgirp commented Nov 28, 2020

Currently, the items are not cleared, removal of items and adding them would result in duplicated items being sold in the shop.

Pull Request Prelude

Changes Proposed

Cleared the last element after the loop.

Issues addressed:

@dastgirp dastgirp changed the base branch from stable to master November 28, 2020 12:37
@AnnieRuru
Copy link
Contributor

prontera,162,175,3	trader	Headgears Merchant 1	1_F_MERCHANT_01,{
OnInit:
	sellitem Ribbon_;
	stopselling Ribbon_;
	sellitem Ribbon_;
	stopselling Ribbon_;
	sellitem Ribbon_;
	stopselling Ribbon_;
	sellitem Ribbon_;
}

how to reproduce ? this ribbon only shows up 1 time on my test server

@dastgirp
Copy link
Member Author

dastgirp commented Nov 29, 2020

Try this:

prontera,162,175,3	trader	Headgears Merchant 1	1_F_MERCHANT_01,{
OnInit:
        tradertype(NST_BARTER);
	sellitem 501, 1, 7227, 1;
        sellitem 502, 1, 7227, 1;
        sellitem 503, 1, 7227, 1;
        sellitem 504, 1, 7227, 1;
	stopselling 501, 7227, 1;
        stopselling 502, 7227, 1;
        stopselling 503, 7227, 1;
        stopselling 504, 7227, 1;
	sellitem 1002, 1 7227, 1;
        sellitem 1003, 1 7227, 1;
        sellitem 1004, 1 7227, 1;
}

forgot to mention, I was using barter shop to reproduce it

@AnnieRuru
Copy link
Contributor

nope, still can't reproduce

/* demonstrate barter shop */
prontera,159,160,4	trader	Barter Shop#prt	4_M_KID1,{
OnInit:
	tradertype(NST_BARTER);
	sellitem White_Herb, 100, Red_Potion, 2;
	stopselling White_Herb, Red_Potion, 2;
	sellitem Blue_Herb, 200, Orange_Potion, 3;
	stopselling Blue_Herb, Orange_Potion, 3;
	sellitem Blue_Herb, 10, Orange_Potion, 3;
	end;
}

this sample was from doc\sample\npc_trader_sample.txt

tested everything there on that sample file and seems to be working fine without this PR

@dastgirp
Copy link
Member Author

nope, still can't reproduce

/* demonstrate barter shop */
prontera,159,160,4	trader	Barter Shop#prt	4_M_KID1,{
OnInit:
	tradertype(NST_BARTER);
	sellitem White_Herb, 100, Red_Potion, 2;
	stopselling White_Herb, Red_Potion, 2;
	sellitem Blue_Herb, 200, Orange_Potion, 3;
	stopselling Blue_Herb, Orange_Potion, 3;
	sellitem Blue_Herb, 10, Orange_Potion, 3;
	end;
}

this sample was from doc\sample\npc_trader_sample.txt

tested everything there on that sample file and seems to be working fine without this PR

Did you try the sample provided by me?
Add multiple item, then remove them in the same sequence.
Then add another set of items

@AnnieRuru
Copy link
Contributor

prontera,162,175,3	trader	Headgears Merchant 1	1_F_MERCHANT_01,{
OnInit:
	tradertype(NST_BARTER);
	sellitem 501, 1, 7227, 1;
	sellitem 502, 1, 7227, 1;
	sellitem 503, 1, 7227, 1;
	sellitem 504, 1, 7227, 1;
	stopselling 501, 7227, 1;
	stopselling 502, 7227, 1;
	stopselling 503, 7227, 1;
	stopselling 504, 7227, 1;
	sellitem 1002, 1, 7227, 1;
	sellitem 1003, 1, 7227, 1;
	sellitem 1004, 1, 7227, 1;
}


do I have to post a screenshot as well ?

@MishimaHaruna
Copy link
Member

I think that in order for this bug to occur, the nd->u.scr.shop->item[] list should have gaps (empty entries with item id set to 0), but I can't think of a way to make that happen. Can you reproduce the issue if you load that script on a mapserver just started and without prior errors on the console, @dastgirp ?

If there is indeed a way to create gaps in the list, then I believe this function shouldn't just clear nd->u.scr.shop->item[cursor], but every index >= cursor and < i

@dastgirp
Copy link
Member Author

I will have to test it more. As this was done with little old source and customized one. It might be possible it's not happening in new hercules.

Will let you know the result.

@MishimaHaruna MishimaHaruna marked this pull request as draft January 11, 2021 11:51
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.

3 participants