Skip to content

Added firemaking plugin for Kotlin#349

Open
tlf30 wants to merge 7 commits into
apollo-rsps:kotlin-experimentsfrom
tlf30:kotlin-experiments-firemaking
Open

Added firemaking plugin for Kotlin#349
tlf30 wants to merge 7 commits into
apollo-rsps:kotlin-experimentsfrom
tlf30:kotlin-experiments-firemaking

Conversation

@tlf30

@tlf30 tlf30 commented Sep 10, 2017

Copy link
Copy Markdown
Contributor

Hello.
I have ported the firemaking plugin as discussed in #303. This has several fixes in it and I did test it.
The only things I am not sure on are the random equations for a successful chance of lighting the fire and how long the fire should burn.

EDIT: Also, this does not support lighting logs already on the ground because I have no way to test easily until dropping items is implemented. Also does not enforce not lighting fires in banks as I am not sure how to implement that.

Can someone please review.

PS: Yes, I know I have a lot of pull requests open right now. Sorry.

Thank you,
Trevor

}

fun walkCoords(direction: Direction): Position {
if (direction == Direction.NORTH) {

@garyttierney garyttierney Sep 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

position.step(1, direction))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is much simpler, thank you!

@garyttierney

Copy link
Copy Markdown
Contributor

PS: Yes, I know I have a lot of pull requests open right now. Sorry.

No, that's nothing to be sorry about. It's great :-). Will get around to reviewing shortly.

@tlf30 tlf30 force-pushed the kotlin-experiments-firemaking branch from 927c273 to 15ba396 Compare September 17, 2017 00:08
@tlf30

tlf30 commented Sep 17, 2017

Copy link
Copy Markdown
Contributor Author

This has been rebased and setup for the new build system for kotlin plugins.

Fix wrong class path
}


on { ItemOnItemMessage::class }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Major- didn't you write something in Ruby to handle these ItemOnItem pairs?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I removed it because it wasn't very good, maybe we could look into something for kotlin

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is the ItemOnItemMessage not the correct way to do this?

@Major- Major- Sep 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No this is correct, we're just discussing how we can make it a bit nicer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, ok

@Major- Major- added the blocked Blocked on another issue label Sep 18, 2017
@Major-

Major- commented Sep 18, 2017

Copy link
Copy Markdown
Member

Blocked on item dropping

@Major-

Major- commented Sep 24, 2017

Copy link
Copy Markdown
Member

After Firemaking is merged we're going to freeze kotlin-experiments for new content until it reaches master (i.e. we'll only be merging plugin ports, not new plugins). Any new content is welcome, but it will sit in PR until we're ready or stuff we still need to port will never get done.

@Major- Major- added plugin and removed kotlin labels Sep 24, 2017
@rmcmk

rmcmk commented Sep 30, 2017

Copy link
Copy Markdown
Contributor

@tlf30 I'm working on the ground item code now.

@tlf30

tlf30 commented Nov 15, 2017

Copy link
Copy Markdown
Contributor Author

@ryleykimmel your ground item code is awesome! Once it is merged in I update this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Blocked on another issue plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants