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

Fix field attribute parsing in items.xml #2510

Merged
merged 1 commit into from Oct 10, 2018
Merged

Fix field attribute parsing in items.xml #2510

merged 1 commit into from Oct 10, 2018

Conversation

marksamman
Copy link
Member

The values of ticks, start and count were always discarded because they're reset on next iteration unless I'm missing something. I'm not sure how/if this ever worked, please have a look that this does not have unintended side-effects, I suspect that some users may have written their items.xml with workarounds for this problem. "start > 0" would never be true when reading the damage key.

@marksamman
Copy link
Member Author

Looks like this was broken in #2481.

@ubap
Copy link

ubap commented Oct 9, 2018

Looks good code wise but can't test it right now

@DSpeichert
Copy link
Member

Indeed it looks like these changes match the original way it worked, here's the commit part that broke it:
709fb34#diff-cdbcf3a5db86a021022fed1a9f317741L805

It's not entirely clear at first what's the intended algorithm here, but after analyzing that I noticed that most likely for a field item definition like this:

	<item id="1492" article="a" name="fire field">
		<attribute key="type" value="magicfield" />
		<attribute key="replaceable" value="0" />
		<attribute key="field" value="fire">
			<attribute key="damage" value="20" />
			<attribute key="ticks" value="10000" />
			<attribute key="count" value="7" />
			<attribute key="damage" value="10" />
		</attribute>
	</item>

The idea is that ticks and count applies to the second damage (=10). The first damage attribute is not a mistake but is added with implied 0 ticks and 1 count.
between #2481 and this PR, we were losing the start, ticks, and count attributes, always resetting them like @marksamman noted.

@DSpeichert DSpeichert merged commit b88a13a into otland:master Oct 10, 2018
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.

None yet

3 participants