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

Basic Implementation for Towers #1

Merged
merged 8 commits into from Aug 11, 2019

Conversation

@darshan3
Copy link
Member

commented Jul 21, 2019

Project Cards:

  1. Towers [1]

How to test:

  1. Start a new game with Core or Builder Gameplay mode.
  2. Give yourself a Tower Root block through command give towerRoot
  3. Place it and interact with it.
  4. Activate the tower

Expected outcome:
A screen should appear with an activation checkbox. Click it to activate the towers. A prefab should appear above the root block to indicate that it is activated.

@skaldarnar
Copy link

left a comment

Please look into the testRook.prefab and whether it is really needed. Otherwise the code looks good, documentation could also be added in a follow-up PR.

"entity" : {
"prefab" : "Towers:towerRoot"
},
"attachmentAllowed": false,

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jul 30, 2019

Can one build on top of a tower root block? Do the towers in Gooey's Defence support multi-block variants?

This comment has been minimized.

Copy link
@darshan3

darshan3 Aug 5, 2019

Author Member

Yes, towers in GooeyDefence support multi-block variants, that is, you can place blocks on each other. "attachmentAllowed": false will not allow that. But if I don't keep that, then the prefab that appears on activation will overlap with the other block placed on the root block. Please suggest what should I do.

@@ -0,0 +1,11 @@
{
"skeletalmesh" : {
"mesh" : "rook",

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jul 30, 2019

Where does this mesh come from (Light and Shadow resources I suppose…). Is the test prefab even needed?

This comment has been minimized.

Copy link
@darshan3

darshan3 Aug 5, 2019

Author Member

Yes, it comes from L&S Resources. It was just used for testing purposes as I didn't have any tower related prefab available. I will remove it once this module is ready.

import org.terasology.network.ServerEvent;

@ServerEvent
public class SendTowerActivationRequest implements Event {

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jul 30, 2019

Is the event the activation request or is this event the request to sent a request? I'm wondering about the naming, I would have expected something like ActivateTowerRequest implements Event 🤔

This comment has been minimized.

Copy link
@darshan3

darshan3 Aug 5, 2019

Author Member

Yup, it is an activation request. I have renamed it as you have suggested.

}
}

private String getActionId(long id) {

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jul 30, 2019

Would be nice to have some explanation in form of JavaDoc here. At least a global description of what a Tower is supposed to, how it functions, and which lifecycle events it has. General documentation of the API can also be added to the README or module wiki…

This comment has been minimized.

Copy link
@darshan3

darshan3 Aug 5, 2019

Author Member

Yup, I would add it once basic implementation is done. I was concerned about changes in the code and implementation post review, hence I have just added code for now. Once implementation is final, I will add javadocs everywhere as well as how to use this module in a README file.

import org.terasology.world.block.ForceBlockActive;

@ForceBlockActive
public class TowerComponent implements Component {

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jul 30, 2019

Again, JavaDoc is highly appreciated (but can be added in a later PR).

This comment has been minimized.

Copy link
@darshan3

darshan3 Aug 5, 2019

Author Member

Same as the previous comment.

@darshan3 darshan3 force-pushed the develop branch from 7915754 to a5157ed Aug 5, 2019

@iaronaraujo iaronaraujo merged commit 348635e into master Aug 11, 2019

@skaldarnar skaldarnar deleted the develop branch Aug 13, 2019

@skaldarnar skaldarnar restored the develop branch Aug 13, 2019

@skaldarnar skaldarnar deleted the develop branch Aug 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.